| Message ID | 20230315110546.2518305-7-msp@baylibre.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | can: m_can: Optimizations for m_can/tcan part 2 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject |
On Wed, Mar 15, 2023 at 12:05:36PM +0100, Markus Schneider-Pargmann wrote: > Combine header and data before writing to the transmit fifo to reduce > the overhead for peripheral chips. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> Thanks for addressing my comments on v2. > --- > drivers/net/can/m_can/m_can.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > index a5003435802b..35a2332464e5 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -1681,6 +1681,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) > m_can_write(cdev, M_CAN_TXBAR, 0x1); > /* End of xmit function for version 3.0.x */ > } else { > + char buf[TXB_ELEMENT_SIZE]; > + u8 len_padded = DIV_ROUND_UP(cf->len, 4); > /* Transmit routine for version >= v3.1.x */ > > txfqs = m_can_read(cdev, M_CAN_TXFQS); > @@ -1720,12 +1722,11 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) > fifo_header.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) | > FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) | > fdflags | TX_BUF_EFC; > - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, &fifo_header, 2); > - if (err) > - goto out_fail; > + memcpy(buf, &fifo_header, 8); > + memcpy_and_pad(&buf[8], len_padded, &cf->data, cf->len, 0); I'm probably missing something obvious here but I'm seeing: * len_padded is the number of 4-byte words * but the 2nd argument to memcpy_and_pad should be a length in bytes * so perhaps it should be: len_padded * 4 > > - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_DATA, > - cf->data, DIV_ROUND_UP(cf->len, 4)); > + err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, > + buf, 2 + len_padded); This part looks good to me :) > if (err) > goto out_fail; > > -- > 2.39.2 >
On 15.03.2023 12:05:36, Markus Schneider-Pargmann wrote: > Combine header and data before writing to the transmit fifo to reduce > the overhead for peripheral chips. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > --- > drivers/net/can/m_can/m_can.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > index a5003435802b..35a2332464e5 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -1681,6 +1681,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) > m_can_write(cdev, M_CAN_TXBAR, 0x1); > /* End of xmit function for version 3.0.x */ > } else { > + char buf[TXB_ELEMENT_SIZE]; Can you create a proper struct that describes a single FIFO entry (id, dlc, data)? Use that struct instead of the struct id_and_dlc fifo_header. > + u8 len_padded = DIV_ROUND_UP(cf->len, 4); > /* Transmit routine for version >= v3.1.x */ > > txfqs = m_can_read(cdev, M_CAN_TXFQS); > @@ -1720,12 +1722,11 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) > fifo_header.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) | > FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) | > fdflags | TX_BUF_EFC; > - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, &fifo_header, 2); > - if (err) > - goto out_fail; > + memcpy(buf, &fifo_header, 8); > + memcpy_and_pad(&buf[8], len_padded, &cf->data, cf->len, 0); > > - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_DATA, > - cf->data, DIV_ROUND_UP(cf->len, 4)); > + err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, > + buf, 2 + len_padded); > if (err) > goto out_fail; Marc
Hi Simon, On Thu, Mar 16, 2023 at 10:27:41AM +0100, Simon Horman wrote: > On Wed, Mar 15, 2023 at 12:05:36PM +0100, Markus Schneider-Pargmann wrote: > > Combine header and data before writing to the transmit fifo to reduce > > the overhead for peripheral chips. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > > Thanks for addressing my comments on v2. > > > --- > > drivers/net/can/m_can/m_can.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > > index a5003435802b..35a2332464e5 100644 > > --- a/drivers/net/can/m_can/m_can.c > > +++ b/drivers/net/can/m_can/m_can.c > > @@ -1681,6 +1681,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) > > m_can_write(cdev, M_CAN_TXBAR, 0x1); > > /* End of xmit function for version 3.0.x */ > > } else { > > + char buf[TXB_ELEMENT_SIZE]; > > + u8 len_padded = DIV_ROUND_UP(cf->len, 4); > > /* Transmit routine for version >= v3.1.x */ > > > > txfqs = m_can_read(cdev, M_CAN_TXFQS); > > @@ -1720,12 +1722,11 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) > > fifo_header.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) | > > FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) | > > fdflags | TX_BUF_EFC; > > - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, &fifo_header, 2); > > - if (err) > > - goto out_fail; > > + memcpy(buf, &fifo_header, 8); > > + memcpy_and_pad(&buf[8], len_padded, &cf->data, cf->len, 0); > > I'm probably missing something obvious here but I'm seeing: > > * len_padded is the number of 4-byte words > * but the 2nd argument to memcpy_and_pad should be a length in bytes > * so perhaps it should be: len_padded * 4 Thank you Simon for all the reviews, finally some time to continue on this: Thanks for pointing this out. I updated my script used for testing so I catch something like this the next time. I will be using TXB_ELEMENT_SIZE - 8 to reflect the buffer size and the 8 byte offset. Best, Markus > > > > > - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_DATA, > > - cf->data, DIV_ROUND_UP(cf->len, 4)); > > + err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, > > + buf, 2 + len_padded); > > This part looks good to me :) > > > if (err) > > goto out_fail; > > > > -- > > 2.39.2 > >
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index a5003435802b..35a2332464e5 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1681,6 +1681,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) m_can_write(cdev, M_CAN_TXBAR, 0x1); /* End of xmit function for version 3.0.x */ } else { + char buf[TXB_ELEMENT_SIZE]; + u8 len_padded = DIV_ROUND_UP(cf->len, 4); /* Transmit routine for version >= v3.1.x */ txfqs = m_can_read(cdev, M_CAN_TXFQS); @@ -1720,12 +1722,11 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) fifo_header.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) | FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) | fdflags | TX_BUF_EFC; - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, &fifo_header, 2); - if (err) - goto out_fail; + memcpy(buf, &fifo_header, 8); + memcpy_and_pad(&buf[8], len_padded, &cf->data, cf->len, 0); - err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_DATA, - cf->data, DIV_ROUND_UP(cf->len, 4)); + err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, + buf, 2 + len_padded); if (err) goto out_fail;
Combine header and data before writing to the transmit fifo to reduce the overhead for peripheral chips. Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> --- drivers/net/can/m_can/m_can.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)