> -----Original Message----- > From: Marc Kleine-Budde [mailto:mkl@xxxxxxxxxxxxxx] > Sent: Monday, July 30, 2018 8:21 PM > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>; linux-can@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 5/6] net: can: flexcan: split the Message Buffer RAM > area > > 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? This tells the number of Message Buffers in each RAM block. suppose Block0 is configured to 8 bytes payload, Block1 to 16 bytes, than the following table indicates how the Message Buffers will be arranged into RAM. RAM partition example ________________________________________________________________________________ RAM block | Payload size | Number of MBs in the RAM block | Message Buffer range 0 | 8 | 32 | 0 to 31 1 | 16 | 21 |32 to 52 -------------------------------------------------------------------------------- > > > 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 *)®s->mb + (mb_size > * n)); > > + if (n < priv->mb_count_block1) > > + mb = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->mb1 + (mb_size * n)); > > + else > > + mb = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->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. This is a good suggestion. I will incorporate it in V2 > > > > > 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 *)®s->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 *)®s->mb + (mb_size * i)); > > + ((u8 *)®s->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 *)®s->mb + (mb_size * i)); > > + if (i < priv->mb_count_block1) > > + mb = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->mb1 + (mb_size * i)); > > + else > > + mb = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->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 *)®s->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 *)®s->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 *)®s->mb + (mb_size * priv->tx_mb_idx)); > > + > > + if (priv->tx_mb_idx < priv->mb_count_block1) > > + priv->tx_mb = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->mb1 + (mb_size * priv- > >tx_mb_idx)); > > + else > > + priv->tx_mb = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->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 *)®s->mb1 + > > + (mb_size * tx_mb_reserved_idx)); > > + else > > + priv->tx_mb_reserved = (struct flexcan_mb __iomem *) > > + ((u8 *)®s->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 | ��.n��������+%������w��{.n�����{����* jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
