[U-Boot] [PATCH v2] drivers: net: cpsw: always flush cache of size PKTSIZE_ALIGN

cpsw tries to flush dcache which is not in the range of PKTSIZE. Because of this the following warning comes while flushing:
CACHE: Misaligned operation at range [dffecec0, dffed016]
Fix it by flushing cache of size PKTSIZE_ALIGN as similar to what is being done in _cpsw_recv.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- Changes since v1: - Use PKTALIGN instead of cache line size - Need not align start of packet buffer from the network subsystem.
drivers/net/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 2ce4ec6..8ce5716 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -907,7 +907,7 @@ static int _cpsw_send(struct cpsw_priv *priv, void *packet, int length) int timeout = CPDMA_TIMEOUT;
flush_dcache_range((unsigned long)packet, - (unsigned long)packet + length); + (unsigned long)packet + PKTSIZE_ALIGN);
/* first reap completed packets */ while (timeout-- &&

On Tuesday 09 August 2016 11:17 AM, Lokesh Vutla wrote:
cpsw tries to flush dcache which is not in the range of PKTSIZE. Because of this the following warning comes while flushing:
CACHE: Misaligned operation at range [dffecec0, dffed016]
Fix it by flushing cache of size PKTSIZE_ALIGN as similar to what is being done in _cpsw_recv.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Tested-by: Mugunthan V N mugunthanvnm@ti.com
Regards Mugunthan V N

Hi Lokesh
On Tue, Aug 9, 2016 at 12:47 AM, Lokesh Vutla lokeshvutla@ti.com wrote:
cpsw tries to flush dcache which is not in the range of PKTSIZE. Because of this the following warning comes while flushing:
CACHE: Misaligned operation at range [dffecec0, dffed016]
Fix it by flushing cache of size PKTSIZE_ALIGN as similar to what is being done in _cpsw_recv.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Changes since v1:
- Use PKTALIGN instead of cache line size
- Need not align start of packet buffer from the network subsystem.
drivers/net/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 2ce4ec6..8ce5716 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -907,7 +907,7 @@ static int _cpsw_send(struct cpsw_priv *priv, void *packet, int length) int timeout = CPDMA_TIMEOUT;
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
(unsigned long)packet + PKTSIZE_ALIGN);
Technically you are flushing more than needed since you just need to be aligned to the cacheline beyond the size being sent, but you are flushing the maximum packet size every time. That said, it's still going to work and the code is readable. I'm fine with this, but wanted to make sure you knew this is more flushing than needed.
Acked-by: Joe Hershberger joe.hershberger@ni.com
/* first reap completed packets */ while (timeout-- &&
-- 2.9.2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Wednesday 10 August 2016 08:43 AM, Joe Hershberger wrote:
Hi Lokesh
On Tue, Aug 9, 2016 at 12:47 AM, Lokesh Vutla lokeshvutla@ti.com wrote:
cpsw tries to flush dcache which is not in the range of PKTSIZE. Because of this the following warning comes while flushing:
CACHE: Misaligned operation at range [dffecec0, dffed016]
Fix it by flushing cache of size PKTSIZE_ALIGN as similar to what is being done in _cpsw_recv.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Changes since v1:
- Use PKTALIGN instead of cache line size
- Need not align start of packet buffer from the network subsystem.
drivers/net/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 2ce4ec6..8ce5716 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -907,7 +907,7 @@ static int _cpsw_send(struct cpsw_priv *priv, void *packet, int length) int timeout = CPDMA_TIMEOUT;
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
(unsigned long)packet + PKTSIZE_ALIGN);
Technically you are flushing more than needed since you just need to be aligned to the cacheline beyond the size being sent, but you are flushing the maximum packet size every time. That said, it's still going to work and the code is readable. I'm fine with this, but wanted to make sure you knew this is more flushing than needed.
Initially that was my idea as well and aligned length to cache line size. As you suggested to use PKTSIZE_ALIGN and also _cpsw_recv uses the same. So, I used PKTSIZE_ALIGN.
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thanks and regards, Lokesh
/* first reap completed packets */ while (timeout-- &&
-- 2.9.2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Lokesh,
On Wed, Aug 10, 2016 at 7:02 AM, Lokesh Vutla lokeshvutla@ti.com wrote:
On Wednesday 10 August 2016 08:43 AM, Joe Hershberger wrote:
Hi Lokesh
On Tue, Aug 9, 2016 at 12:47 AM, Lokesh Vutla lokeshvutla@ti.com wrote:
cpsw tries to flush dcache which is not in the range of PKTSIZE. Because of this the following warning comes while flushing:
CACHE: Misaligned operation at range [dffecec0, dffed016]
Fix it by flushing cache of size PKTSIZE_ALIGN as similar to what is being done in _cpsw_recv.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Changes since v1:
- Use PKTALIGN instead of cache line size
- Need not align start of packet buffer from the network subsystem.
drivers/net/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 2ce4ec6..8ce5716 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -907,7 +907,7 @@ static int _cpsw_send(struct cpsw_priv *priv, void *packet, int length) int timeout = CPDMA_TIMEOUT;
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
(unsigned long)packet + PKTSIZE_ALIGN);
Technically you are flushing more than needed since you just need to be aligned to the cacheline beyond the size being sent, but you are flushing the maximum packet size every time. That said, it's still going to work and the code is readable. I'm fine with this, but wanted to make sure you knew this is more flushing than needed.
Initially that was my idea as well and aligned length to cache line size. As you suggested to use PKTSIZE_ALIGN and also _cpsw_recv uses the same. So, I used PKTSIZE_ALIGN.
I actually recommended PKTALIGN, not PKTSIZE_ALIGN. For recv, the size for invalidate is unknown until it's received (unless you can check a descriptor after it's received but before you read it), so it's more appropriate to use PKTSIZE_ALIGN. For send, you certainly know the size before flushing.
Cheers, -Joe

On Thursday 11 August 2016 03:11 AM, Joe Hershberger wrote:
Hi Lokesh,
On Wed, Aug 10, 2016 at 7:02 AM, Lokesh Vutla lokeshvutla@ti.com wrote:
On Wednesday 10 August 2016 08:43 AM, Joe Hershberger wrote:
Hi Lokesh
On Tue, Aug 9, 2016 at 12:47 AM, Lokesh Vutla lokeshvutla@ti.com wrote:
cpsw tries to flush dcache which is not in the range of PKTSIZE. Because of this the following warning comes while flushing:
CACHE: Misaligned operation at range [dffecec0, dffed016]
Fix it by flushing cache of size PKTSIZE_ALIGN as similar to what is being done in _cpsw_recv.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Changes since v1:
- Use PKTALIGN instead of cache line size
- Need not align start of packet buffer from the network subsystem.
drivers/net/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 2ce4ec6..8ce5716 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -907,7 +907,7 @@ static int _cpsw_send(struct cpsw_priv *priv, void *packet, int length) int timeout = CPDMA_TIMEOUT;
flush_dcache_range((unsigned long)packet,
(unsigned long)packet + length);
(unsigned long)packet + PKTSIZE_ALIGN);
Technically you are flushing more than needed since you just need to be aligned to the cacheline beyond the size being sent, but you are flushing the maximum packet size every time. That said, it's still going to work and the code is readable. I'm fine with this, but wanted to make sure you knew this is more flushing than needed.
Initially that was my idea as well and aligned length to cache line size. As you suggested to use PKTSIZE_ALIGN and also _cpsw_recv uses the same. So, I used PKTSIZE_ALIGN.
I actually recommended PKTALIGN, not PKTSIZE_ALIGN. For recv, the size
Oops my bad. I totally misunderstood your previous comment. Ill resend this patch.
Thanks and regards, Lokesh
for invalidate is unknown until it's received (unless you can check a descriptor after it's received but before you read it), so it's more appropriate to use PKTSIZE_ALIGN. For send, you certainly know the size before flushing.
Cheers, -Joe
participants (3)
-
Joe Hershberger
-
Lokesh Vutla
-
Mugunthan V N