Add support for queueing SKBs to the host over the transfer ring of the
relevant channel. The mhi_ep_queue_skb() API will be used by the client
networking drivers to queue the SKBs to the host over MHI bus.
The host will add ring elements to the transfer ring periodically for
the device and the device will write SKBs to the ring elements. If a
single SKB doesn't fit in a ring element (TRE), it will be placed in
multiple ring elements and the overflow event will be sent for all ring
elements except the last one. For the last ring element, the EOT event
will be sent indicating the packet boundary.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/bus/mhi/ep/main.c | 102 ++++++++++++++++++++++++++++++++++++++
include/linux/mhi_ep.h | 13 +++++
2 files changed, 115 insertions(+)
diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index baf383a4857b..e4186b012257 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -488,6 +488,108 @@ int mhi_ep_process_tre_ring(struct mhi_ep_ring *ring, struct mhi_ep_ring_element
return 0;
}
+int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir,
+ struct sk_buff *skb, size_t len, enum mhi_flags mflags)
+{
+ struct mhi_ep_chan *mhi_chan = (dir == DMA_FROM_DEVICE) ? mhi_dev->dl_chan :
+ mhi_dev->ul_chan;
+ struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
+ struct device *dev = &mhi_chan->mhi_dev->dev;
+ struct mhi_ep_ring_element *el;
+ struct mhi_ep_ring *ring;
+ size_t bytes_to_write;
+ enum mhi_ev_ccs code;
+ void *read_from_loc;
+ u32 buf_remaining;
+ u64 write_to_loc;
+ u32 tre_len;
+ int ret = 0;
+
+ if (dir == DMA_TO_DEVICE)
+ return -EINVAL;
+
+ buf_remaining = len;
+ ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
+
+ mutex_lock(&mhi_chan->lock);
+
+ do {
+ /* Don't process the transfer ring if the channel is not in RUNNING state */
+ if (mhi_chan->state != MHI_CH_STATE_RUNNING) {
+ dev_err(dev, "Channel not available\n");
+ ret = -ENODEV;
+ goto err_exit;
+ }
+
+ if (mhi_ep_queue_is_empty(mhi_dev, dir)) {
+ dev_err(dev, "TRE not available!\n");
+ ret = -EINVAL;
+ goto err_exit;
+ }
+
+ el = &ring->ring_cache[ring->rd_offset];
+ tre_len = MHI_EP_TRE_GET_LEN(el);
+ if (skb->len > tre_len) {
+ dev_err(dev, "Buffer size (%d) is too large for TRE (%d)!\n",
+ skb->len, tre_len);
+ ret = -ENOMEM;
+ goto err_exit;
+ }
+
+ bytes_to_write = min(buf_remaining, tre_len);
+ read_from_loc = skb->data;
+ write_to_loc = MHI_EP_TRE_GET_PTR(el);
+
+ ret = mhi_cntrl->write_to_host(mhi_cntrl, read_from_loc, write_to_loc,
+ bytes_to_write);
+ if (ret < 0)
+ goto err_exit;
+
+ buf_remaining -= bytes_to_write;
+ /*
+ * For all TREs queued by the host for DL channel, only the EOT flag will be set.
+ * If the packet doesn't fit into a single TRE, send the OVERFLOW event to
+ * the host so that the host can adjust the packet boundary to next TREs. Else send
+ * the EOT event to the host indicating the packet boundary.
+ */
+ if (buf_remaining)
+ code = MHI_EV_CC_OVERFLOW;
+ else
+ code = MHI_EV_CC_EOT;
+
+ ret = mhi_ep_send_completion_event(mhi_cntrl, ring, bytes_to_write, code);
+ if (ret) {
+ dev_err(dev, "Error sending completion event\n");
+ goto err_exit;
+ }
+
+ mhi_ep_ring_inc_index(ring);
+ } while (buf_remaining);
+
+ /*
+ * During high network traffic, sometimes the DL doorbell interrupt from the host is missed
+ * by the endpoint. So manually check for the write pointer update here so that we don't run
+ * out of buffer due to missing interrupts.
+ */
+ if (ring->rd_offset + 1 == ring->wr_offset) {
+ ret = mhi_ep_update_wr_offset(ring);
+ if (ret) {
+ dev_err(dev, "Error updating write pointer\n");
+ goto err_exit;
+ }
+ }
+
+ mutex_unlock(&mhi_chan->lock);
+
+ return 0;
+
+err_exit:
+ mutex_unlock(&mhi_chan->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mhi_ep_queue_skb);
+
static int mhi_ep_cache_host_cfg(struct mhi_ep_cntrl *mhi_cntrl)
{
struct device *dev = &mhi_cntrl->mhi_dev->dev;
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index aaf4b6942037..75cfbf0c6fb0 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -277,4 +277,17 @@ void mhi_ep_power_down(struct mhi_ep_cntrl *mhi_cntrl);
*/
bool mhi_ep_queue_is_empty(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir);
+/**
+ * mhi_ep_queue_skb - Send SKBs to host over MHI Endpoint
+ * @mhi_dev: Device associated with the channels
+ * @dir: DMA direction for the channel
+ * @skb: Buffer for holding SKBs
+ * @len: Buffer length
+ * @mflags: MHI Endpoint transfer flags used for the transfer
+ *
+ * Return: 0 if the SKBs has been sent successfully, a negative error code otherwise.
+ */
+int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir,
+ struct sk_buff *skb, size_t len, enum mhi_flags mflags);
+
#endif
--
2.25.1
On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
> Add support for queueing SKBs to the host over the transfer ring of the
> relevant channel. The mhi_ep_queue_skb() API will be used by the client
> networking drivers to queue the SKBs to the host over MHI bus.
>
> The host will add ring elements to the transfer ring periodically for
> the device and the device will write SKBs to the ring elements. If a
> single SKB doesn't fit in a ring element (TRE), it will be placed in
> multiple ring elements and the overflow event will be sent for all ring
> elements except the last one. For the last ring element, the EOT event
> will be sent indicating the packet boundary.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
I'm a little confused by this, so maybe you can provide
a better explanation somehow.
-Alex
> ---
> drivers/bus/mhi/ep/main.c | 102 ++++++++++++++++++++++++++++++++++++++
> include/linux/mhi_ep.h | 13 +++++
> 2 files changed, 115 insertions(+)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index baf383a4857b..e4186b012257 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -488,6 +488,108 @@ int mhi_ep_process_tre_ring(struct mhi_ep_ring *ring, struct mhi_ep_ring_element
> return 0;
> }
>
> +int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir,
> + struct sk_buff *skb, size_t len, enum mhi_flags mflags)
Why are both skb and len supplied? Will an skb be supplied
without wanting to send all of it? Must len be less than
skb->len? I'm a little confused about the interface.
Also, the data direction is *out*, right? You'll never
be queueing a "receive" SKB?
> +{
> + struct mhi_ep_chan *mhi_chan = (dir == DMA_FROM_DEVICE) ? mhi_dev->dl_chan :
> + mhi_dev->ul_chan;
> + struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
> + struct device *dev = &mhi_chan->mhi_dev->dev;
> + struct mhi_ep_ring_element *el;
> + struct mhi_ep_ring *ring;
> + size_t bytes_to_write;
> + enum mhi_ev_ccs code;
> + void *read_from_loc;
> + u32 buf_remaining;
> + u64 write_to_loc;
> + u32 tre_len;
> + int ret = 0;
> +
> + if (dir == DMA_TO_DEVICE)
> + return -EINVAL;
Can't you just preclude this from happening, or
know it won't happen by inspection?
> +
> + buf_remaining = len;
> + ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
> +
> + mutex_lock(&mhi_chan->lock);
> +
> + do {
> + /* Don't process the transfer ring if the channel is not in RUNNING state */
> + if (mhi_chan->state != MHI_CH_STATE_RUNNING) {
> + dev_err(dev, "Channel not available\n");
> + ret = -ENODEV;
> + goto err_exit;
> + }
> +
It would be nice if the caller could know whether there
was enough room *before* you start transferring things.
It's probably a lot of work to get to that point though.
> + if (mhi_ep_queue_is_empty(mhi_dev, dir)) {
> + dev_err(dev, "TRE not available!\n");
> + ret = -EINVAL;
> + goto err_exit;
> + }
> +
> + el = &ring->ring_cache[ring->rd_offset];
> + tre_len = MHI_EP_TRE_GET_LEN(el);
> + if (skb->len > tre_len) {
> + dev_err(dev, "Buffer size (%d) is too large for TRE (%d)!\n",
> + skb->len, tre_len);
This means the receive buffer must be big enough to hold
any incoming SKB. This is *without* checking for the
CHAIN flag in the TRE, so what you describe in the
patch description seems not to be true. I.e., multiple
TREs in a TRD will *not* be consumed if the SKB data
requires more than what's left in the current TRE.
But you have some other code below, so it's likely I'm
just misunderstanding this.
> + ret = -ENOMEM;
> + goto err_exit;
> + }
> +
> + bytes_to_write = min(buf_remaining, tre_len);
> + read_from_loc = skb->data;
> + write_to_loc = MHI_EP_TRE_GET_PTR(el);
> +
> + ret = mhi_cntrl->write_to_host(mhi_cntrl, read_from_loc, write_to_loc,
> + bytes_to_write);
> + if (ret < 0)
> + goto err_exit;
> +
> + buf_remaining -= bytes_to_write;
> + /*
> + * For all TREs queued by the host for DL channel, only the EOT flag will be set.
> + * If the packet doesn't fit into a single TRE, send the OVERFLOW event to
> + * the host so that the host can adjust the packet boundary to next TREs. Else send
> + * the EOT event to the host indicating the packet boundary.
> + */
> + if (buf_remaining)
> + code = MHI_EV_CC_OVERFLOW;
> + else
> + code = MHI_EV_CC_EOT;
> +
> + ret = mhi_ep_send_completion_event(mhi_cntrl, ring, bytes_to_write, code);
> + if (ret) {
> + dev_err(dev, "Error sending completion event\n");
> + goto err_exit;
> + }
> +
> + mhi_ep_ring_inc_index(ring);
> + } while (buf_remaining);
> +
> + /*
> + * During high network traffic, sometimes the DL doorbell interrupt from the host is missed
> + * by the endpoint. So manually check for the write pointer update here so that we don't run
> + * out of buffer due to missing interrupts.
> + */
> + if (ring->rd_offset + 1 == ring->wr_offset) {
> + ret = mhi_ep_update_wr_offset(ring);
> + if (ret) {
> + dev_err(dev, "Error updating write pointer\n");
> + goto err_exit;
> + }
> + }
> +
> + mutex_unlock(&mhi_chan->lock);
> +
> + return 0;
> +
> +err_exit:
> + mutex_unlock(&mhi_chan->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(mhi_ep_queue_skb);
> +
> static int mhi_ep_cache_host_cfg(struct mhi_ep_cntrl *mhi_cntrl)
> {
> struct device *dev = &mhi_cntrl->mhi_dev->dev;
> diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
> index aaf4b6942037..75cfbf0c6fb0 100644
> --- a/include/linux/mhi_ep.h
> +++ b/include/linux/mhi_ep.h
> @@ -277,4 +277,17 @@ void mhi_ep_power_down(struct mhi_ep_cntrl *mhi_cntrl);
> */
> bool mhi_ep_queue_is_empty(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir);
>
> +/**
> + * mhi_ep_queue_skb - Send SKBs to host over MHI Endpoint
> + * @mhi_dev: Device associated with the channels
> + * @dir: DMA direction for the channel
> + * @skb: Buffer for holding SKBs
> + * @len: Buffer length
> + * @mflags: MHI Endpoint transfer flags used for the transfer
> + *
> + * Return: 0 if the SKBs has been sent successfully, a negative error code otherwise.
> + */
> +int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir,
> + struct sk_buff *skb, size_t len, enum mhi_flags mflags);
> +
> #endif
On 2/22/22 9:18 AM, Alex Elder wrote:
>
>> But anyway, there is no need to check for CHAIN flag while writing to
>> host.
>> CHAIN flag is only used or even make sense when host writes data to
>> device, so
>
> I'm not sure that's correct, but I don't want to get into that issue here.
> We can talk about that separately.
I just wanted to send a short followup here. My comments
were based on a misunderstanding, and Mani cleared it up
for me. For host receives, the MHI specification states
that a packet that requires more than the size of the
buffer in a single TRE leads to an overflow event being
generated from the device to the host. The buffer on
the TRE is filled, and subsequent packet data is written
to the next TRE's buffer (assuming it's present).
This differs from one feature of IPA and its GSI transfer
rings. I won't explain that here, to avoid any confusion.
Mani explained things to me, and he's going to send an
updated series, which I'll review.
-Alex
On 2/22/22 8:38 AM, Manivannan Sadhasivam wrote:
> On Tue, Feb 15, 2022 at 04:40:29PM -0600, Alex Elder wrote:
>> On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
>>> Add support for queueing SKBs to the host over the transfer ring of the
>>> relevant channel. The mhi_ep_queue_skb() API will be used by the client
>>> networking drivers to queue the SKBs to the host over MHI bus.
>>>
>>> The host will add ring elements to the transfer ring periodically for
>>> the device and the device will write SKBs to the ring elements. If a
>>> single SKB doesn't fit in a ring element (TRE), it will be placed in
>>> multiple ring elements and the overflow event will be sent for all ring
>>> elements except the last one. For the last ring element, the EOT event
>>> will be sent indicating the packet boundary.
>>>
>>> Signed-off-by: Manivannan Sadhasivam <[email protected]>
>>
>> I'm a little confused by this, so maybe you can provide
>> a better explanation somehow.
>>
>> -Alex
>>
>>> ---
>>> drivers/bus/mhi/ep/main.c | 102 ++++++++++++++++++++++++++++++++++++++
>>> include/linux/mhi_ep.h | 13 +++++
>>> 2 files changed, 115 insertions(+)
>>>
>>> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
>>> index baf383a4857b..e4186b012257 100644
>>> --- a/drivers/bus/mhi/ep/main.c
>>> +++ b/drivers/bus/mhi/ep/main.c
>>> @@ -488,6 +488,108 @@ int mhi_ep_process_tre_ring(struct mhi_ep_ring *ring, struct mhi_ep_ring_element
>>> return 0;
>>> }
>>> +int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir,
>>> + struct sk_buff *skb, size_t len, enum mhi_flags mflags)
>>
>> Why are both skb and len supplied? Will an skb be supplied
>> without wanting to send all of it? Must len be less than
>> skb->len? I'm a little confused about the interface.
>>
>> Also, the data direction is *out*, right? You'll never
>> be queueing a "receive" SKB?
>>
>
> This was done to be compatible with the MHI host API where the host can queue
> SKBs in both directions. But I think I should stop doing this.
OK.
>>> +{
>>> + struct mhi_ep_chan *mhi_chan = (dir == DMA_FROM_DEVICE) ? mhi_dev->dl_chan :
>>> + mhi_dev->ul_chan;
>>> + struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
>>> + struct device *dev = &mhi_chan->mhi_dev->dev;
>>> + struct mhi_ep_ring_element *el;
>>> + struct mhi_ep_ring *ring;
>>> + size_t bytes_to_write;
>>> + enum mhi_ev_ccs code;
>>> + void *read_from_loc;
>>> + u32 buf_remaining;
>>> + u64 write_to_loc;
>>> + u32 tre_len;
>>> + int ret = 0;
>>> +
>>> + if (dir == DMA_TO_DEVICE)
>>> + return -EINVAL;
>>
>> Can't you just preclude this from happening, or
>> know it won't happen by inspection?
>>
>>> +
>>> + buf_remaining = len;
>>> + ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
>>> +
>>> + mutex_lock(&mhi_chan->lock);
>>> +
>>> + do {
>>> + /* Don't process the transfer ring if the channel is not in RUNNING state */
>>> + if (mhi_chan->state != MHI_CH_STATE_RUNNING) {
>>> + dev_err(dev, "Channel not available\n");
>>> + ret = -ENODEV;
>>> + goto err_exit;
>>> + }
>>> +
>>
>> It would be nice if the caller could know whether there
>> was enough room *before* you start transferring things.
>> It's probably a lot of work to get to that point though.
>>
>
> No, the caller will do this check but the check is included here so that we
> don't run out of buffers when the packet needs to be splitted.
>
>>> + if (mhi_ep_queue_is_empty(mhi_dev, dir)) {
>>> + dev_err(dev, "TRE not available!\n");
>>> + ret = -EINVAL;
>>> + goto err_exit;
>>> + }
>>> +
>>> + el = &ring->ring_cache[ring->rd_offset];
>>> + tre_len = MHI_EP_TRE_GET_LEN(el);
>>> + if (skb->len > tre_len) {
>>> + dev_err(dev, "Buffer size (%d) is too large for TRE (%d)!\n",
>>> + skb->len, tre_len);
>>
>> This means the receive buffer must be big enough to hold
>> any incoming SKB. This is *without* checking for the
>> CHAIN flag in the TRE, so what you describe in the
>> patch description seems not to be true. I.e., multiple
>> TREs in a TRD will *not* be consumed if the SKB data
>> requires more than what's left in the current TRE.
>>
>
> I think I removed this check for v3 but somehow the change got lost :/
Looking at this now, it's possible I got confused about
which direction the data was moving; but I'm not really
sure.
From the perspective of the endpoint device, this is the
*transmit* function. But when the device is transmitting,
it is moving data into the *receive* buffers that the host
has allocated and supplied via the transfer ring.
My statement seems to be correct though, with this logic,
the host must supply a buffer large enough to receive the
entire next SKB, or it will get an error back. I no longer
know what happens when this function (mhi_ep_queue_skb())
returns an error--is the skb dropped?
> But anyway, there is no need to check for CHAIN flag while writing to host.
> CHAIN flag is only used or even make sense when host writes data to device, so
I'm not sure that's correct, but I don't want to get into that issue here.
We can talk about that separately.
> that it knows the packet boundary and could use the CHAIN flag to tell the
> device where the boundary lies.
This doesn't sound to me like what the purpose of the CHAIN flag is,
but perhaps I'm misunderstanding you. Let's have a quick private
chat about this so we don't waste any more e-mail bandwidth.
-Alex
> But when the device writes to host, it already has the pre-queued elements from
> host that has no idea where the packet boundary lies. So the host would've set
> only EOT on all TREs and expects the device to send OVERFLOW event for TREs that
> don't have the complete packet. Then finally, when device sends EOT event, the
> host will detect the boundary.
>
> Thanks,
> Mani
On Tue, Feb 15, 2022 at 04:40:29PM -0600, Alex Elder wrote:
> On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
> > Add support for queueing SKBs to the host over the transfer ring of the
> > relevant channel. The mhi_ep_queue_skb() API will be used by the client
> > networking drivers to queue the SKBs to the host over MHI bus.
> >
> > The host will add ring elements to the transfer ring periodically for
> > the device and the device will write SKBs to the ring elements. If a
> > single SKB doesn't fit in a ring element (TRE), it will be placed in
> > multiple ring elements and the overflow event will be sent for all ring
> > elements except the last one. For the last ring element, the EOT event
> > will be sent indicating the packet boundary.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
>
> I'm a little confused by this, so maybe you can provide
> a better explanation somehow.
>
> -Alex
>
> > ---
> > drivers/bus/mhi/ep/main.c | 102 ++++++++++++++++++++++++++++++++++++++
> > include/linux/mhi_ep.h | 13 +++++
> > 2 files changed, 115 insertions(+)
> >
> > diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> > index baf383a4857b..e4186b012257 100644
> > --- a/drivers/bus/mhi/ep/main.c
> > +++ b/drivers/bus/mhi/ep/main.c
> > @@ -488,6 +488,108 @@ int mhi_ep_process_tre_ring(struct mhi_ep_ring *ring, struct mhi_ep_ring_element
> > return 0;
> > }
> > +int mhi_ep_queue_skb(struct mhi_ep_device *mhi_dev, enum dma_data_direction dir,
> > + struct sk_buff *skb, size_t len, enum mhi_flags mflags)
>
> Why are both skb and len supplied? Will an skb be supplied
> without wanting to send all of it? Must len be less than
> skb->len? I'm a little confused about the interface.
>
> Also, the data direction is *out*, right? You'll never
> be queueing a "receive" SKB?
>
This was done to be compatible with the MHI host API where the host can queue
SKBs in both directions. But I think I should stop doing this.
> > +{
> > + struct mhi_ep_chan *mhi_chan = (dir == DMA_FROM_DEVICE) ? mhi_dev->dl_chan :
> > + mhi_dev->ul_chan;
> > + struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
> > + struct device *dev = &mhi_chan->mhi_dev->dev;
> > + struct mhi_ep_ring_element *el;
> > + struct mhi_ep_ring *ring;
> > + size_t bytes_to_write;
> > + enum mhi_ev_ccs code;
> > + void *read_from_loc;
> > + u32 buf_remaining;
> > + u64 write_to_loc;
> > + u32 tre_len;
> > + int ret = 0;
> > +
> > + if (dir == DMA_TO_DEVICE)
> > + return -EINVAL;
>
> Can't you just preclude this from happening, or
> know it won't happen by inspection?
>
> > +
> > + buf_remaining = len;
> > + ring = &mhi_cntrl->mhi_chan[mhi_chan->chan].ring;
> > +
> > + mutex_lock(&mhi_chan->lock);
> > +
> > + do {
> > + /* Don't process the transfer ring if the channel is not in RUNNING state */
> > + if (mhi_chan->state != MHI_CH_STATE_RUNNING) {
> > + dev_err(dev, "Channel not available\n");
> > + ret = -ENODEV;
> > + goto err_exit;
> > + }
> > +
>
> It would be nice if the caller could know whether there
> was enough room *before* you start transferring things.
> It's probably a lot of work to get to that point though.
>
No, the caller will do this check but the check is included here so that we
don't run out of buffers when the packet needs to be splitted.
> > + if (mhi_ep_queue_is_empty(mhi_dev, dir)) {
> > + dev_err(dev, "TRE not available!\n");
> > + ret = -EINVAL;
> > + goto err_exit;
> > + }
> > +
> > + el = &ring->ring_cache[ring->rd_offset];
> > + tre_len = MHI_EP_TRE_GET_LEN(el);
> > + if (skb->len > tre_len) {
> > + dev_err(dev, "Buffer size (%d) is too large for TRE (%d)!\n",
> > + skb->len, tre_len);
>
> This means the receive buffer must be big enough to hold
> any incoming SKB. This is *without* checking for the
> CHAIN flag in the TRE, so what you describe in the
> patch description seems not to be true. I.e., multiple
> TREs in a TRD will *not* be consumed if the SKB data
> requires more than what's left in the current TRE.
>
I think I removed this check for v3 but somehow the change got lost :/
But anyway, there is no need to check for CHAIN flag while writing to host.
CHAIN flag is only used or even make sense when host writes data to device, so
that it knows the packet boundary and could use the CHAIN flag to tell the
device where the boundary lies.
But when the device writes to host, it already has the pre-queued elements from
host that has no idea where the packet boundary lies. So the host would've set
only EOT on all TREs and expects the device to send OVERFLOW event for TREs that
don't have the complete packet. Then finally, when device sends EOT event, the
host will detect the boundary.
Thanks,
Mani