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

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

 

On 08/12/2018 06:14 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
    ^^^

space

> bytes.
> 
> Therefore, added provision in the driver for payload size to be 64 bytes.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> ---
> 
> Notes:
>     V4:
>      - change regs->mb from u32 array to u8 array
>      - introduced priv->mb_count to help with loops in flexcan_start
>      - Add WARN_ON in flexcan_get_mb
>      - moved data in xmit and mailbox_read functions to for loop
>      - use DIV_ROUND_UP in xmit and mailbox_read functions
>     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 | 85 +++++++++++++++++++++++++------------
>  1 file changed, 59 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 621085273633..8c7f96f29118 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 */
> +	u8  mb[1024];		/* 0x80 */
>  	/* FIFO-mode:
>  	 *			MB
>  	 * 0x080...0x08f	0	RX message buffer
> @@ -262,6 +262,7 @@ struct flexcan_priv {
>  	struct flexcan_mb __iomem *tx_mb;
>  	struct flexcan_mb __iomem *tx_mb_reserved;
>  	u8 tx_mb_idx;
> +	u8 mb_count;
>  	u32 reg_ctrl_default;
>  	u32 reg_imask1_default;
>  	u32 reg_imask2_default;
> @@ -353,6 +354,23 @@ 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;
> +
> +	if (WARN_ON(mb_index >= priv->mb_count))
> +		return NULL;
> +
> +	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;

Can you cache the value for mb_size in flexcan_open()?

> +
> +	return (struct flexcan_mb __iomem *)
> +		(&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;
> @@ -517,8 +535,8 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
>  	const struct flexcan_priv *priv = netdev_priv(dev);
>  	struct can_frame *cf = (struct can_frame *)skb->data;
>  	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 +553,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 = DIV_ROUND_UP(cf->can_dlc, sizeof(u32));
> +	for (i = 0; i < can_dlc_dword; i++) {
> +		u32 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 +681,11 @@ 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;
> +	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 +726,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 = DIV_ROUND_UP(cf->can_dlc, sizeof(u32));
> +	for (i = 0; i < can_dlc_dword; i++) {
> +		__be32 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) {
> @@ -907,6 +928,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);
> @@ -988,15 +1010,18 @@ 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);
> +	for (i = priv->tx_mb_idx; i < priv->mb_count; i++) {
> +		mb = flexcan_get_mb(priv, i);
> +		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
>  	}
>  
>  	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 */
> @@ -1016,7 +1041,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 < priv->mb_count; i++)
>  		priv->write(0, &regs->rximr[i]);
>  
>  	/* On Vybrid, disable memory error detection interrupts
> @@ -1099,8 +1124,8 @@ static void flexcan_chip_stop(struct net_device *dev)
>  static int flexcan_open(struct net_device *dev)
>  {
>  	struct flexcan_priv *priv = netdev_priv(dev);
> -	struct flexcan_regs __iomem *regs = priv->regs;
>  	int err;
> +	u8 mb_size;
>  
>  	err = clk_prepare_enable(priv->clk_ipg);
>  	if (err)
> @@ -1118,14 +1143,22 @@ 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;

Please cache mb_size here, too.

> +	priv->mb_count = sizeof(priv->regs->mb) / mb_size;
> +
>  	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;
> @@ -1136,7 +1169,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 = priv->mb_count - 1;
>  
>  		imask = GENMASK_ULL(priv->offload.mb_last,
>  				    priv->offload.mb_first);
> 

Looks good otherwise.

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