net: fec: Avoid MX28 bus sync issue

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Quoted 5 lines · Marek Vasut

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

Quoted 15 lines · Marek Vasut

Quoted 13 lines

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Quoted 4 lines

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

It's working here too.

Tested-by Alexandre Pereira da Silva <[email protected]>

Hi,

Quoted 19 lines · Fabio Estevam

Quoted 18 lines · Marek Vasut

Quoted 5 lines

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Quoted 5 lines

Quoted 1 line

Tested-by Fabio Estevam <[email protected]>

It's working here too.

Tested-by Alexandre Pereira da Silva <[email protected]>

Nice to hear, thank Albert for finding this.

Thanks for sharing.

Unfortunately I'm still seeing non-recoverable timeouts when doing tftp transfers.
Nevertheless, with this patch sometimes I'm able to transfer big files (100MiB) without problems (I was never able before). So this is a big improvement.
I applied this patch over a v2013.01, does it need any additional patch? I think I saw one email about flush dcache...

Dear Marek,

Quoted 21 lines · Marek Vasut

Hi,

Quoted 19 lines · Fabio Estevam

Quoted 18 lines · Marek Vasut

Quoted 5 lines

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Tested-by Fabio Estevam <[email protected]>

It's working here too.

Tested-by Alexandre Pereira da Silva <[email protected]>

Nice to hear, thank Albert for finding this.

Thanks for sharing.

Unfortunately I'm still seeing non-recoverable timeouts when doing tftp transfers. Nevertheless, with this patch sometimes I'm able to transfer big files (100MiB) without problems (I was never able before). So this is a big improvement. I applied this patch over a v2013.01, does it need any additional patch? I think I saw one email about flush dcache...

Considering the other guys seem to work without problems I guess this scenario is specific to my board. I'm using a Micrel KSZ8031RNLI at 50MHz. I always suspect from the PHY.

I'm disconcerted because usually the timeouts occur after having successfully downloaded 22 or 24 MiB. Other times it just downloads completely.

In any case, good job!

Best regards,
--
Hector Palacios

Hi Hector,

Quoted 34 lines

Dear Marek,

Quoted 21 lines · Marek Vasut

Hi,

Quoted 19 lines · Fabio Estevam

Quoted 18 lines · Marek Vasut

Quoted 5 lines

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Tested-by Fabio Estevam <[email protected]>

It's working here too.

Tested-by Alexandre Pereira da Silva <[email protected]>

Nice to hear, thank Albert for finding this.

Thanks for sharing.

Quoted 5 lines

Unfortunately I'm still seeing non-recoverable timeouts when doing tftp transfers. Nevertheless, with this patch sometimes I'm able to transfer big files (100MiB) without problems (I was never able before). So this is a big improvement. I applied this patch over a v2013.01, does it need any additional patch? I think I saw one email about flush dcache...

Try Stefano's tree as Fabio suggested. I think it's already pushed and includes the fixes.

Quoted 3 lines

Considering the other guys seem to work without problems I guess this scenario is specific to my board. I'm using a Micrel KSZ8031RNLI at 50MHz. I always suspect from the PHY.

You can try using the PHYLIB (CONFIG_PHYLIB and CONFIG_PHY_SMSC as in sc_sps_1.h ) . Also, can you check which of the two "ret = -EINVAL" is triggered in fec_send() ? You can add simple printf() alongside both of them.

Hi Marek,

Quoted 36 lines · Marek Vasut

Hi Hector,

Quoted 34 lines

Dear Marek,

Quoted 21 lines · Marek Vasut

Hi,

Quoted 19 lines · Fabio Estevam

Quoted 18 lines · Marek Vasut

Quoted 5 lines

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Tested-by Fabio Estevam <[email protected]>

It's working here too.

Tested-by Alexandre Pereira da Silva <[email protected]>

Nice to hear, thank Albert for finding this.

Thanks for sharing.

Quoted 8 lines

Quoted 5 lines

Unfortunately I'm still seeing non-recoverable timeouts when doing tftp transfers. Nevertheless, with this patch sometimes I'm able to transfer big files (100MiB) without problems (I was never able before). So this is a big improvement. I applied this patch over a v2013.01, does it need any additional patch? I think I saw one email about flush dcache...

Try Stefano's tree as Fabio suggested. I think it's already pushed and includes the fixes.

I just tried, but it didn't help.

Quoted 7 lines

Quoted 3 lines

Considering the other guys seem to work without problems I guess this scenario is specific to my board. I'm using a Micrel KSZ8031RNLI at 50MHz. I always suspect from the PHY.

You can try using the PHYLIB (CONFIG_PHYLIB and CONFIG_PHY_SMSC as in sc_sps_1.h ) . Also, can you check which of the two "ret = -EINVAL" is triggered in fec_send() ? You can add simple printf() alongside both of them.

fec_send() *does not* ever fail, but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command.
I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.
I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

Best regards,
--
Hector Palacios

Quoted 2 lines

Try Stefano's tree as Fabio suggested. I think it's already pushed and includes the fixes.

I just tried, but it didn't help.

Quoted 8 lines

Quoted 3 lines

Considering the other guys seem to work without problems I guess this scenario is specific to my board. I'm using a Micrel KSZ8031RNLI at 50MHz. I always suspect from the PHY.

You can try using the PHYLIB (CONFIG_PHYLIB and CONFIG_PHY_SMSC as in sc_sps_1.h ) . Also, can you check which of the two "ret = -EINVAL" is triggered in fec_send() ? You can add simple printf() alongside both of them.

fec_send() *does not* ever fail

but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command. I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.

I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

Quoted 11 lines

[...]

but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command. I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.

Holy moly!

Quoted 4 lines

I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

LAN8720 phy, right? Try implementing something like [1], by clearing the EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a simple PHY register RMW which you can stick somewhere into the PHY net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine/+/b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0

Hi Marek,

Quoted 20 lines · Marek Vasut

Quoted 12 lines

[...]

but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command. I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.

Holy moly!

Quoted 4 lines

I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

Quoted 8 lines

LAN8720 phy, right? Try implementing something like [1], by clearing the EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a simple PHY register RMW which you can stick somewhere into the PHY net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine/+
/b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0

No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting but I checked the PHY registers and EDPD mode (Energy Detect Power Down) is off, at least before running the tftp command. Power Down mode is off too, so unless these are somehow enabled during the TFTP process, this is not what's happening.

The sniffer shows that the TFTP server simply stops sending data packets. I can see however the target sending several times the ACK packet to the last received data packet. This would point to the TFTP server (as Albert suggested), but the fact is the problem occurs with different TFTP servers (I tried three different servers) and it does not happen with an old v2009 U-Boot using the same target.

Many times, though not always, after the timeout occurs, I cancel with CTRL+C and run the tftp command again, and then the file downloads completely.
The PHY is my usual suspect...

Dear Hector Palacios,

Quoted 32 lines

Hi Marek,

Quoted 20 lines · Marek Vasut

Quoted 12 lines

[...]

but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command. I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.

Holy moly!

Quoted 4 lines

I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

Quoted 8 lines

LAN8720 phy, right? Try implementing something like [1], by clearing the EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a simple PHY register RMW which you can stick somewhere into the PHY net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine/+
/b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0

Quoted 7 lines

No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting but I checked the PHY registers and EDPD mode (Energy Detect Power Down) is off, at least before running the tftp command. Power Down mode is off too, so unless these are somehow enabled during the TFTP process, this is not what's happening.

OK, makes sense.

Quoted 6 lines

The sniffer shows that the TFTP server simply stops sending data packets. I can see however the target sending several times the ACK packet to the last received data packet. This would point to the TFTP server (as Albert suggested), but the fact is the problem occurs with different TFTP servers (I tried three different servers) and it does not happen with an old v2009 U-Boot using the same target.

Can you try running "dcache off" command before running the TFTP transfer? Does it still behave like this?

You might need to define #define CONFIG_CMD_CACHE for this to work.

Hi Marek,

Quoted 51 lines · Marek Vasut

Dear Hector Palacios,

Quoted 32 lines

Hi Marek,

Quoted 20 lines · Marek Vasut

Quoted 12 lines

[...]

but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command. I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.

Holy moly!

Quoted 4 lines

I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

Quoted 8 lines

LAN8720 phy, right? Try implementing something like [1], by clearing the EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a simple PHY register RMW which you can stick somewhere into the PHY net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine
/+ /b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0

Quoted 7 lines

No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting but I checked the PHY registers and EDPD mode (Energy Detect Power Down) is off, at least before running the tftp command. Power Down mode is off too, so unless these are somehow enabled during the TFTP process, this is not what's happening.

OK, makes sense.

Quoted 6 lines

The sniffer shows that the TFTP server simply stops sending data packets. I can see however the target sending several times the ACK packet to the last received data packet. This would point to the TFTP server (as Albert suggested), but the fact is the problem occurs with different TFTP servers (I tried three different servers) and it does not happen with an old v2009 U-Boot using the same target.

Quoted 4 lines

Can you try running "dcache off" command before running the TFTP transfer? Does it still behave like this?

You might need to define #define CONFIG_CMD_CACHE for this to work.

Sourcery!
It's not that it works with dcache off, I found something even more
strange: The way I reproduce this issue is by setting the 'bootcmd' to 'dboot ${loadaddr} file100M'. When you set the 'bootcmd' like this:

	setenv bootcmd tftp ${loadaddr} file100M

this eventually expands to

	bootcmd=tftp 0x42000000 file100M

So this is the command that runs automatically after the bootdelay. I just discovered that if instead of letting the auto boot run, I press a key to stop the auto boot and run the command by hand (either running 'boot' or typing the command 'tftp 0x42000000 file100M'), the tftp transfer works perfectly.
On the other hand, if I do the same but use the environment variable ${loadaddr}, i.e. 'tftp ${loadaddr} file100M'. It will stop after 10 seconds.

And to make things funnier I just reproduced this issue on the MX28EVK using the imx U-Boot custodian tree at commit a3f170cdbf7ae0bd24c94c2f46725699bbd69f05. That discards being a platform issue.

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M'

in
your EVK? If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).
As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Best regards,
--
Hector Palacios

Hi Hector,

Quoted 70 lines

Hi Marek,

Quoted 51 lines · Marek Vasut

Dear Hector Palacios,

Quoted 32 lines

Hi Marek,

Quoted 20 lines · Marek Vasut

Quoted 12 lines

   [...]

but I found something:
It is very strange that the timeouts appear always after transferring between 20 and 24 MiB. So I thought maybe it was not an issue with the size of the file or the number of packets received, but instead a timed issue (an issue that happens after some period of time). I checked, and in fact the timeouts occur exactly 10 seconds after running the tftp command. I verified that this is what is happening by adding a udelay(100000) at fec_send(). In this case, the timeout also occurs after 10 seconds, but due to the delay, I have transferred only a few Kbytes.

Holy moly!

Quoted 4 lines

I tried to change different timeout related constants at tftp.c but still the issue happens after 10s.
It's like if, after these 10 seconds, the PHY lost the link or something. Really odd. Does it tell you anything?

Quoted 8 lines

LAN8720 phy, right? Try implementing something like [1], by clearing the EDPWRDOWN bit , the PHY will never enter low-power mode. It's just a simple PHY register RMW which you can stick somewhere into the PHY net/phy/smsc.c code.

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/djbw/dmaengine
/+ /b629820d18fa65cc598390e4b9712fd5f83ee693%5E!/#F0

Quoted 7 lines

No, my PHY is a Micrel KSZ8031RNLI.

The hint about the PHY possibly going to power down mode is interesting but I checked the PHY registers and EDPD mode (Energy Detect Power Down) is off, at least before running the tftp command. Power Down mode is off too, so unless these are somehow enabled during the TFTP process, this is not what's happening.

OK, makes sense.

Quoted 6 lines

The sniffer shows that the TFTP server simply stops sending data packets. I can see however the target sending several times the ACK packet to the last received data packet. This would point to the TFTP server (as Albert suggested), but the fact is the problem occurs with different TFTP servers (I tried three different servers) and it does not happen with an old v2009 U-Boot using the same target.

Quoted 4 lines

Can you try running "dcache off" command before running the TFTP transfer? Does it still behave like this?

You might need to define #define CONFIG_CMD_CACHE for this to work.

Sourcery!
It's not that it works with dcache off, I found something even more
strange: The way I reproduce this issue is by setting the 'bootcmd' to 'dboot ${loadaddr} file100M'. When you set the 'bootcmd' like this:

	setenv bootcmd tftp ${loadaddr} file100M

this eventually expands to

	bootcmd=tftp 0x42000000 file100M

Quoted 8 lines

So this is the command that runs automatically after the bootdelay. I just discovered that if instead of letting the auto boot run, I press a key to stop the auto boot and run the command by hand (either running 'boot' or typing the command 'tftp 0x42000000 file100M'), the tftp transfer works perfectly.
On the other hand, if I do the same but use the environment variable ${loadaddr}, i.e. 'tftp ${loadaddr} file100M'. It will stop after 10 seconds.

Count it be your hardware needs some more delay to stabilize?

Quoted 4 lines

And to make things funnier I just reproduced this issue on the MX28EVK using the imx U-Boot custodian tree at commit a3f170cdbf7ae0bd24c94c2f46725699bbd69f05. That discards being a platform issue.

It still might be a PHY issue, no?

Quoted 1 line

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M'

I guess that'd be a 100MB file?

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Quoted 2 lines · Hector Palacios

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Quoted 11 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

Quoted 6 lines

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

This is my tftp configuration for your reference:

$ cat /etc/xinetd.d/tftp
        service tftp
        {
        protocol        = udp
        port            = 69
        socket_type     = dgram
        wait            = yes
        user            = nobody
        server          = /usr/sbin/in.tftpd
        server_args     = /tftpboot
        disable         = no
        }

Dear Fabio Estevam,

Quoted 18 lines · Fabio Estevam

Quoted 17 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Another thing of interest would be a 'tcpdump' pcap capture of that connection.

Dear Marek,

Quoted 20 lines · Marek Vasut

Dear Fabio Estevam,

Quoted 18 lines · Fabio Estevam

Quoted 17 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Yes I can.

Quoted 2 lines

Another thing of interest would be a 'tcpdump' pcap capture of that connection.

I was initially filtering out only TFTP packets of my wireshark trace and all looked correct. After taking a second look to the full trace I see now a hint. Around 7 seconds after starting the TFTP transfer the server is sending an ARP to the target asking for the owner of the target's IP. The target is receiving this ARP and apparently responding (at least this is what my debug code shows as it gets into arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP reply from the target is not reaching the network. My sniffer does not capture this reply.

The server resends the ARP request twice more (seconds 8 and 9) to the target and since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who has that IP. Since nobody responds it stops sending data.

The times that it works (and I don't know the magic behind using a numeric address versus using ${loadaddr} when they have the same value), the ARP replies do reach the network and the server continues the transmission normally.

Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP replies always reach the network, and the transfers always succeed.

Since Fabio cannot reproduce it I guess it must be a local ghost. :o(

Dear Marek,

Quoted 20 lines · Marek Vasut

Dear Fabio Estevam,

Quoted 18 lines · Fabio Estevam

Quoted 17 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Yes I can.

Quoted 1 line

Another thing of interest would be a 'tcpdump' pcap capture of that connection.

I was initially filtering out only TFTP packets of my wireshark trace and all looked correct. After taking a second look to the full trace I see now a hint. Around 7 seconds after starting the TFTP transfer the server is sending an ARP to the target asking for the owner of the target's IP. The target is receiving this ARP and apparently responding (at least this is what my debug code shows as it gets into arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP reply from the target is not reaching the network. My sniffer does not capture this reply.

The server resends the ARP request twice more (seconds 8 and 9) to the target and since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who has that IP. Since nobody responds it stops sending data.

The times that it works (and I don't know the magic behind using a numeric address versus using ${loadaddr} when they have the same value), the ARP replies do reach the network and the server continues the transmission normally.

Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP replies always reach the network, and the transfers always succeed.

Since Fabio cannot reproduce it I guess it must be a local ghost. :o(

Hello,

Going back to this old thread I have some news regarding the problem with TFTP transmissions blocking (timed out) after 10 seconds on the FEC of the MX28. See below:

Quoted 6 lines · Hector Palacios

Dear Marek,

Quoted 3 lines · Marek Vasut

Dear Fabio Estevam,

Quoted 1 line

On Tue, Jul 16, 2013 at 12:51 AM, Fabio Estevam <[email protected]>

Quoted 43 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Yes I can.

Quoted 2 lines

Another thing of interest would be a 'tcpdump' pcap capture of that connection.

I was initially filtering out only TFTP packets of my wireshark trace and all looked correct. After taking a second look to the full trace I see now a hint. Around 7 seconds after starting the TFTP transfer the server is sending an ARP to the target asking for the owner of the target's IP. The target is receiving this ARP and apparently responding (at least this is what my debug code shows as it gets into arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP reply from the target is not reaching the network. My sniffer does not capture this reply.

The server resends the ARP request twice more (seconds 8 and 9) to the target and since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who has that IP. Since nobody responds it stops sending data.

The times that it works (and I don't know the magic behind using a numeric address versus using ${loadaddr} when they have the same value), the ARP replies do reach the network and the server continues the transmission normally.

Quoted 4 lines

Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP replies always reach the network, and the transfers always succeed.

Since Fabio cannot reproduce it I guess it must be a local ghost. :o(

We tracked down the issue to an ARP request from the server that was never answered by the target. We later noticed that the problem did not happen anymore when building U-Boot with a different toolchain and that the issue seemed to be in the alignment of the RX buffer in the stack, which old GCC compilers seem to do wrong.

Here is a patch:

From: Robert Hodaszi <[email protected]>
Date: Fri, 6 Sep 2013 09:50:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because of GCC bug

Older GCC versions don't handle well alignment on stack variables. The temporary RX buffer is a local variable, so it is on the stack. Because the FEC driver is using DMA for transmission, receive and transmit buffers should be aligned on 64 byte. The transmit buffers are not allocated in the driver internally, it sends the packets directly as it gets them. So these packets should be aligned.
When the ARP layer wants to reply to an ARP request, it uses the FEC driver's temporary RX buffer (used to pass data to the ARP layer) to store the new packet, and pass it back to the FEC driver's send function. Because of a GCC bug

this buffer is not aligned well, and when the
driver tries to send it, it first rounds the address down to the alignment boundary. That causes invalid data.

To fix it, don't put the temporary onto the stack.

Signed-off-by Robert Hodaszi <[email protected]>

Signed-off-by Hector Palacios <[email protected]>

---
  drivers/net/fec_mxc.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

drivers/net/fec_mxc.c +1 -1

index f4f72b7..315017e 100644
@@ -828,7 +828,10 @@ static int fec_recv(struct eth_device *dev)
         uint16_t bd_status;
         uint32_t addr, size, end;
         int i;
-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Don't place this variable on the stack, because older GCC

version + * doesn't handle alignement on stack well.

<html>
<head>

    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Hi,<br>
      <br>
      There are a lot of bug announcement, just make a search:<br>
      <meta http-equiv="content-type" content="text/html;
        charset=ISO-8859-1">
      <a
href="http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33721">http://gcc.gnu.or
g/bugzilla/show_bug.cgi?id=33721</a><br> <meta http-equiv="content-type"
content="text/html;
        charset=ISO-8859-1">
      <a

href="http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16660">http://gcc.gnu.or g/bugzilla/show_bug.cgi?id=16660</a><br> <br>
Also, I printed out the buffer addresses, and that temporary RX buffer was not aligned. So the transmit function rounded it down to the alignment boundary, and so caused invalid data transmission. (By the way. Shouldn't the transmit function check whether the alignment is proper, and throw an error message, instead of round it down? That would make more sense.)<br> <div class="moz-signature"><br>

Best regards,<br>
Robert Hodaszi
</div>
<br>
On 2013-09-12 12:50, Marek Vasut wrote:<br>
</div>
<blockquote cite="mid:[email protected]" type="cite">
<pre wrap="">Dear Hector Palacios,

Dear Hector Palacios,

Quoted 13 lines

Hello,

Going back to this old thread I have some news regarding the problem with TFTP transmissions blocking (timed out) after 10 seconds on the FEC of the MX28. See below:

Quoted 6 lines · Hector Palacios

Dear Marek,

Quoted 3 lines · Marek Vasut

Dear Fabio Estevam,

Quoted 1 line

On Tue, Jul 16, 2013 at 12:51 AM, Fabio Estevam<[email protected]>

Quoted 55 lines · wrote:

Quoted 38 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Yes I can.

Quoted 2 lines

Another thing of interest would be a 'tcpdump' pcap capture of that connection.

I was initially filtering out only TFTP packets of my wireshark trace and all looked correct. After taking a second look to the full trace I see now a hint. Around 7 seconds after starting the TFTP transfer the server is sending an ARP to the target asking for the owner of the target's IP. The target is receiving this ARP and apparently responding (at least this is what my debug code shows as it gets into arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP reply from the target is not reaching the network. My sniffer does not capture this reply.

The server resends the ARP request twice more (seconds 8 and 9) to the target and since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who has that IP. Since nobody responds it stops sending data.

The times that it works (and I don't know the magic behind using a numeric address versus using ${loadaddr} when they have the same value), the ARP replies do reach the network and the server continues the transmission normally.

Quoted 4 lines

Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP replies always reach the network, and the transfers always succeed.

Since Fabio cannot reproduce it I guess it must be a local ghost. :o(

We tracked down the issue to an ARP request from the server that was never answered by the target. We later noticed that the problem did not happen anymore when building U-Boot with a different toolchain and that the issue seemed to be in the alignment of the RX buffer in the stack, which old GCC compilers seem to do wrong.

Here is a patch:

From: Robert Hodaszi<[email protected]>
Date: Fri, 6 Sep 2013 09:50:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because of GCC bug

Quoted 10 lines

Older GCC versions don't handle well alignment on stack variables. The temporary RX buffer is a local variable, so it is on the stack. Because the FEC driver is using DMA for transmission, receive and transmit buffers should be aligned on 64 byte. The transmit buffers are not allocated in the driver internally, it sends the packets directly as it gets them. So these packets should be aligned.
When the ARP layer wants to reply to an ARP request, it uses the FEC driver's temporary RX buffer (used to pass data to the ARP layer) to store the new packet, and pass it back to the FEC driver's send function. Because of a GCC bug

Can you point to this GCC bug in the GCC bugzilla or something?

Quoted 23 lines

this buffer is not aligned well, and when the
driver tries to send it, it first rounds the address down to the alignment boundary. That causes invalid data.

To fix it, don't put the temporary onto the stack.

Signed-off-by Robert Hodaszi<[email protected]>

Signed-off-by Hector Palacios<[email protected]>

---
   drivers/net/fec_mxc.c | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-)

drivers/net/fec_mxc.c +1 -1

index f4f72b7..315017e 100644
@@ -828,7 +828,10 @@ static int fec_recv(struct eth_device *dev)
          uint16_t bd_status;
          uint32_t addr, size, end;
          int i;
-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Don't place this variable on the stack, because older GCC

version + * doesn't handle alignement on stack well.

drivers/net/fec_mxc.c static int fec_recv(struct eth_device *dev) · line 834

Quoted 2 lines

drivers/net/fec_mxc.c +2 -0

+        */
+       static uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);

The buffer might as well be allocated using ALLOC_CACHE_ALIGN_BUFFER() from include/common.h . Still, are you _really_ sure the buffer is unaligned ? Do you have a testcase maybe ?

btw. I am able to replicate this issue sometimes even using GCC 4.8.0 .

Best regards,
Marek Vasut

Hi,

Sorry, hopefully that will be a plain-text.

There are a lot of bug announcement, just make a search:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33721

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16660

Also, I printed out the buffer addresses, and that temporary RX buffer was not aligned. So the transmit function rounded it down to the alignment boundary, and so caused invalid data transmission. (By the way. Shouldn't the transmit function check whether the alignment is proper, and throw an error message, instead of round it down? That would make more sense.)

Dear Robert Hodaszi,

Quoted 6 lines

Hi,

Sorry, hopefully that will be a plain-text.

There are a lot of bug announcement, just make a search:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33721

This was apparently fixed three years ago.

Quoted 1 line

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16660

And this one six years ago ...

Quoted 6 lines

Also, I printed out the buffer addresses, and that temporary RX buffer was not aligned. So the transmit function rounded it down to the alignment boundary, and so caused invalid data transmission. (By the way. Shouldn't the transmit function check whether the alignment is proper, and throw an error message, instead of round it down? That would make more sense.)

Looking at the code one more time, it'd make most sense to simply allocate the buffer NOT on stack, but with some memalign-kind-of call to avoid this abuse of stack. You see, the max packet size is around 2k, which is quite a lot. How does this proposal sound to you ?

Best regards,
Marek Vasut

I just brought up two samples, that it was a long-term problem. Now it is fixed. But if somebody is trying to compile the U-Boot with an older toolchain, it could cause problems. I had problems with 4.4.6.

Yes, 2k is a lot, and I would not put it on the stack. That was just a quick fix. It would be nicer to allocate the memory with malloc, and should do that at initialization time.

Dear Robert Hodaszi,

Please do not top-post.

Memalign should do here with proper rounding. Can you prepare another patch please?

Best regards,
Marek Vasut

Quoted 4 lines

Dear Robert Hodaszi,

Please do not top-post.

Quoted 5 lines

Memalign should do here with proper rounding. Can you prepare another patch please?

Best regards,
Marek Vasut

Ok. I will try to do that tomorrow.

Looking at the code one more time, it'd make most sense to simply allocate the buffer NOT on stack, but with some memalign-kind-of call to avoid this abuse of stack. You see, the max packet size is around 2k, which is quite a lot. How does this proposal sound to you ?

Dear Marek Vasut,

Quoted 5 lines · In message <[email protected]> you wrote:

Looking at the code one more time, it'd make most sense to simply allocate the buffer NOT on stack, but with some memalign-kind-of call to avoid this abuse of stack. You see, the max packet size is around 2k, which is quite a lot. How does this proposal sound to you ?

It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.

Quoted 3 lines

It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.

This buffer in the fec driver will be used in DMA transfer, so maybe that's the reason we should use malloc instead of using the stack?

At least in the kernel, we don't use stack for DMA buffers.

Dear Fabio Estevam,

Quoted 6 lines · In message <CAOMZO5BY6kDOCoWn_SRwhPE7ssMjAReZ2bcFXGgFF-_Wrdgo0g@mail.gmail.com>…

Quoted 3 lines

It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.

This buffer in the fec driver will be used in DMA transfer, so maybe that's the reason we should use malloc instead of using the stack?

What has DMA to do with that? We're talking about alignment only.

Quoted 8 lines · Wolfgang Denk

Dear Fabio Estevam,

Quoted 4 lines · In message <CAOMZO5BY6kDOCoWn_SRwhPE7ssMjAReZ2bcFXGgFF-_Wrdgo0g@mail.gmail.com>…

Quoted 2 lines

It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.

This buffer in the fec driver will be used in DMA transfer, so maybe that's the reason we should use malloc instead of using the stack?

What has DMA to do with that? We're talking about alignment only.

I mentioned DMA because we align the buffer with __aligned(ARCH_DMA_MINALIGN).

Will try to see if I can reproduce the problem here, but the last time I tried I was not able to.

Maybe the gcc version that Robert and Hector pointed out may explain the different behaviour.

Regards,

Fabio Estevam

Quoted 17 lines · Fabio Estevam

Quoted 10 lines · Wolfgang Denk

Dear Fabio Estevam,

In message <CAOMZO5BY6kDOCoWn_SRwhPE7ssMjAReZ2bcFXGgFF-_Wrdgo0g@mail.gmail.com>

Quoted 4 lines · you

Quoted 2 lines

It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.

This buffer in the fec driver will be used in DMA transfer, so maybe that's the reason we should use malloc instead of using the stack?

What has DMA to do with that? We're talking about alignment only.

I mentioned DMA because we align the buffer with
__aligned(ARCH_DMA_MINALIGN).

Will try to see if I can reproduce the problem here, but the last time I tried I was not able to.

Quoted 6 lines

Maybe the gcc version that Robert and Hector pointed out may explain the different behaviour.

Regards,

Fabio Estevam

Ok. Then what about if I would use the stack, but align the buffer manually.

From: Robert Hodaszi <[email protected]>
Date: Fri, 13 Sep 2013 13:07:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because
of GCC bug

Older GCC versions don't handle well alignment on stack variables. The temporary RX buffer is a local variable, so it is on the stack. Because the FEC driver is using DMA for transmission, receive and transmit buffers should be align on 64 byte. The transmit buffers are not allocated in the driver internally, it sends the packets directly as it gets them. So these packets should be aligned.
When the ARP layer wants to reply to an ARP request, it uses the FEC driver's temporary RX buffer (used to pass data to the ARP layer) to store the new packet, and pass it back to the FEC driver's send function. Because of a GCC bug, this buffer is not aligned well, and when the driver tries to send it, it first rounds the address down to the alignment boundary. That causes invalid data.

To fix it, allocate more space on the stack, and align the buffer manually.

Signed-off-by Robert Hodaszi <[email protected]>

---
 drivers/net/fec_mxc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/net/fec_mxc.c +3 -1

index f4f72b7..09d816d 100644
@@ -828,7 +828,9 @@ static int fec_recv(struct eth_device *dev)
        uint16_t bd_status;
        uint32_t addr, size, end;
        int i;
-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Align the receive buffer */
+       uchar buff_unaligned[FEC_MAX_PKT_SIZE + (ARCH_DMA_MINALIGN - 1)];
+       uchar *buff = ((uint32_t)buff_unaligned + (ARCH_DMA_MINALIGN - 
 1)) & ~(ARCH_DMA_MINALIGN - 1);
 
        /*

* Check if any critical events have happened

Best regards,
Robert Hodaszi

Quoted 77 lines · Fabio Estevam

Quoted 21 lines · Wolfgang Denk

Quoted 5 lines

Dear Fabio Estevam,

In message <CAOMZO5BY6kDOCoWn_SRwhPE7ssMjAReZ2bcFXGgFF-_Wrdgo0g@mail.gmail.com>

Quoted 8 lines

Quoted 5 lines · you

Quoted 2 lines

It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.

This buffer in the fec driver will be used in DMA transfer, so maybe that's the reason we should use malloc instead of using the stack?

What has DMA to do with that? We're talking about alignment only.

I mentioned DMA because we align the buffer with
__aligned(ARCH_DMA_MINALIGN).

Will try to see if I can reproduce the problem here, but the last time I tried I was not able to.

Quoted 6 lines

Maybe the gcc version that Robert and Hector pointed out may explain the different behaviour.

Regards,

Fabio Estevam

Ok. Then what about if I would use the stack, but align the buffer manually.

From: Robert Hodaszi <[email protected]>
Date: Fri, 13 Sep 2013 13:07:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because

of GCC bug

Older GCC versions don't handle well alignment on stack variables. The temporary RX buffer is a local variable, so it is on the stack. Because the FEC driver is using DMA for transmission, receive and transmit buffers should be align on 64 byte. The transmit buffers are not allocated in the driver internally, it sends the packets directly as it gets them. So these packets should be aligned.
When the ARP layer wants to reply to an ARP request, it uses the FEC driver's temporary RX buffer (used to pass data to the ARP layer) to store the new packet, and pass it back to the FEC driver's send function. Because of a GCC bug, this buffer is not aligned well, and when the driver tries to send it, it first rounds the address down to the alignment boundary. That causes invalid data.

To fix it, allocate more space on the stack, and align the buffer manually.

Signed-off-by Robert Hodaszi <[email protected]>

---

 drivers/net/fec_mxc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/net/fec_mxc.c +3 -1

index f4f72b7..09d816d 100644
@@ -828,7 +828,9 @@ static int fec_recv(struct eth_device *dev)
 
        uint16_t bd_status;
        uint32_t addr, size, end;
        int i;
 
-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Align the receive buffer */
+       uchar buff_unaligned[FEC_MAX_PKT_SIZE + (ARCH_DMA_MINALIGN - 1)];
+       uchar *buff = ((uint32_t)buff_unaligned + (ARCH_DMA_MINALIGN -
 1)) & ~(ARCH_DMA_MINALIGN - 1);

Quoted 6 lines

/*

* Check if any critical events have happened

Best regards,
Robert Hodaszi

I think, I sent it again in HTML format... Sorry...

OK, but as Wolfgang already pointed out, can you tell use what compiler exposes this behavior and show us the details WD requested please ?

Best regards,
Marek Vasut

Ok. Then what about if I would use the stack, but align the buffer manually.

Dear Robert Hodaszi,

Quoted 2 lines · In message <[email protected]> you wrote:

Ok. Then what about if I would use the stack, but align the buffer manually.

Has this been tested? Does it work?

Quoted 5 lines

drivers/net/fec_mxc.c +3 -1

-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Align the receive buffer */
+       uchar buff_unaligned[FEC_MAX_PKT_SIZE + (ARCH_DMA_MINALIGN - 1)];
+       uchar *buff = ((uint32_t)buff_unaligned + (ARCH_DMA_MINALIGN -

1)) & ~(ARCH_DMA_MINALIGN - 1);

You should use the ALIGN() macro here.

Quoted 8 lines

Quoted 6 lines

drivers/net/fec_mxc.c +3 -1

-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Align the receive buffer */
+       uchar buff_unaligned[FEC_MAX_PKT_SIZE + (ARCH_DMA_MINALIGN - 1)];
+       uchar *buff = ((uint32_t)buff_unaligned + (ARCH_DMA_MINALIGN -

1)) & ~(ARCH_DMA_MINALIGN - 1);

You should use the ALIGN() macro here.

We already have this stuff in include/common.h ... ALLOC_CACHE_ALIGNED_BUFFER it is called IIRC

That's ALLOC_CACHE_ALIGN_BUFFER. Thanks.

Dear Hector Palacios,

Quoted 13 lines

Hello,

Going back to this old thread I have some news regarding the problem with TFTP transmissions blocking (timed out) after 10 seconds on the FEC of the MX28. See below:

Quoted 6 lines · Hector Palacios

Dear Marek,

Quoted 3 lines · Marek Vasut

Dear Fabio Estevam,

Quoted 1 line

On Tue, Jul 16, 2013 at 12:51 AM, Fabio Estevam<[email protected]>

Quoted 55 lines · wrote:

Quoted 38 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

On Mon, Jul 15, 2013 at 12:09 PM, Hector Palacios

Quoted 2 lines · <[email protected]> wrote:

@Fabio: could you manually run the command 'tftp ${loadaddr} file100M' in your EVK?

Yes, this is what I have been running since the beginning.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

If it doesn't fail, could you try running it again after playing with the environment (setting/printing some variables).

I can't reproduce the problem here.

Quoted 2 lines

As I said, this issue appeared with different TFTP servers and is independent of whether the dcache is or not enabled.

Can you transfer from a PC to another PC via TFTP?

Yes I can.

Quoted 2 lines

Another thing of interest would be a 'tcpdump' pcap capture of that connection.

I was initially filtering out only TFTP packets of my wireshark trace and all looked correct. After taking a second look to the full trace I see now a hint. Around 7 seconds after starting the TFTP transfer the server is sending an ARP to the target asking for the owner of the target's IP. The target is receiving this ARP and apparently responding (at least this is what my debug code shows as it gets into arp.c:ArpReceive(), case ARPOP_REQUEST and sending a packet), but this ARP reply from the target is not reaching the network. My sniffer does not capture this reply.

The server resends the ARP request twice more (seconds 8 and 9) to the target and since it doesn't get a reply then sends a broadcast ARP (seconds 10) asking who has that IP. Since nobody responds it stops sending data.

The times that it works (and I don't know the magic behind using a numeric address versus using ${loadaddr} when they have the same value), the ARP replies do reach the network and the server continues the transmission normally.

Quoted 4 lines

Using a v2009 U-Boot, the behaviour is exactly the same, but the target's ARP replies always reach the network, and the transfers always succeed.

Since Fabio cannot reproduce it I guess it must be a local ghost. :o(

We tracked down the issue to an ARP request from the server that was never answered by the target. We later noticed that the problem did not happen anymore when building U-Boot with a different toolchain and that the issue seemed to be in the alignment of the RX buffer in the stack, which old GCC compilers seem to do wrong.

Here is a patch:

From: Robert Hodaszi<[email protected]>
Date: Fri, 6 Sep 2013 09:50:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because of GCC bug

Quoted 10 lines

Older GCC versions don't handle well alignment on stack variables. The temporary RX buffer is a local variable, so it is on the stack. Because the FEC driver is using DMA for transmission, receive and transmit buffers should be aligned on 64 byte. The transmit buffers are not allocated in the driver internally, it sends the packets directly as it gets them. So these packets should be aligned.
When the ARP layer wants to reply to an ARP request, it uses the FEC driver's temporary RX buffer (used to pass data to the ARP layer) to store the new packet, and pass it back to the FEC driver's send function. Because of a GCC bug

Can you point to this GCC bug in the GCC bugzilla or something?

Quoted 23 lines

this buffer is not aligned well, and when the
driver tries to send it, it first rounds the address down to the alignment boundary. That causes invalid data.

To fix it, don't put the temporary onto the stack.

Signed-off-by Robert Hodaszi<[email protected]>

Signed-off-by Hector Palacios<[email protected]>

---
   drivers/net/fec_mxc.c | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-)

drivers/net/fec_mxc.c +1 -1

index f4f72b7..315017e 100644
@@ -828,7 +828,10 @@ static int fec_recv(struct eth_device *dev)
          uint16_t bd_status;
          uint32_t addr, size, end;
          int i;
-       uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
+       /* Don't place this variable on the stack, because older GCC

version + * doesn't handle alignement on stack well.

drivers/net/fec_mxc.c static int fec_recv(struct eth_device *dev) · line 834

Quoted 2 lines

drivers/net/fec_mxc.c +2 -0

+        */
+       static uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);

The buffer might as well be allocated using ALLOC_CACHE_ALIGN_BUFFER() from include/common.h . Still, are you _really_ sure the buffer is unaligned ? Do you have a testcase maybe ?

btw. I am able to replicate this issue sometimes even using GCC 4.8.0 .

Best regards,
Marek Vasut

Sorry, hopefully that will be a plain-text.

There are a lot of bug announcement, just make a search:
gcc.gnu.org/bugzilla/show_bug.cgi?id=33721 gcc.gnu.org/bugzilla/show_bug.cgi?id=16660

Here is a patch:

From: Robert Hodaszi <[email protected]>
Date: Fri, 6 Sep 2013 09:50:52 +0200
Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because
of GCC bug

Older GCC versions don't handle well alignment on stack variables.

To fix it, don't put the temporary onto the stack.

Signed-off-by Robert Hodaszi <[email protected]>

Signed-off-by Hector Palacios <[email protected]>

---
  drivers/net/fec_mxc.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

drivers/net/fec_mxc.c +0 -0

index f4f72b7..315017e 100644

Quoted 5 lines · Marek Vasut

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 14 lines · Fabio Estevam

Quoted 5 lines · Marek Vasut

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Quoted 9 lines

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Perhaps this because our memory barriers are lacking.

Linux has this code
asm/io.h:#define writel(v,c) ({ __iowmb();
writel_relaxed(v,c); })

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/barrier.h-#elif defined(CONFIG_ARM_DMA_MEM_BUFFERABLE) || defined(CONFIG_SMP)
asm/barrier.h-#define mb() do { dsb(); outer_sync(); }
while (0)
asm/barrier.h-#define rmb() dsb()
asm/barrier.h:#define wmb() mb()
asm/barrier.h-#else

asm/barrier.h-#define mb()              barrier()
asm/barrier.h-#define rmb()             barrier()
asm/barrier.h:#define wmb()             barrier()

asm/barrier.h-#endif

asm/barrier.h-#if __LINUX_ARM_ARCH__ >= 7 asm/barrier.h-#define isb() __asm__ __volatile__ ("isb" : : : "memory") asm/barrier.h:#define dsb() __asm__ __volatile__ ("dsb" : : : "memory") asm/barrier.h-#define dmb() __asm__ __volatile__ ("dmb" : : : "memory") asm/barrier.h-#elif defined(CONFIG_CPU_XSC3) || __LINUX_ARM_ARCH__ == 6 asm/barrier.h-#define isb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h-#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \

_____________________________________
Can you try just adding a dsb() instead of the dummy read?

If this also fixes your problem, it seems better to fix our writel macro

Hi Troy,

Quoted 77 lines · Troy Kisky

Quoted 14 lines · Fabio Estevam

Quoted 5 lines · Marek Vasut

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Quoted 9 lines

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Perhaps this because our memory barriers are lacking.

Linux has this code
asm/io.h:#define writel(v,c) ({ __iowmb();
writel_relaxed(v,c); })

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/barrier.h-#elif defined(CONFIG_ARM_DMA_MEM_BUFFERABLE) || defined(CONFIG_SMP)
asm/barrier.h-#define mb() do { dsb(); outer_sync(); }
while (0)
asm/barrier.h-#define rmb() dsb()
asm/barrier.h:#define wmb() mb()
asm/barrier.h-#else

asm/barrier.h-#define mb()              barrier()
asm/barrier.h-#define rmb()             barrier()
asm/barrier.h:#define wmb()             barrier()

asm/barrier.h-#endif

asm/barrier.h-#if __LINUX_ARM_ARCH__ >= 7 asm/barrier.h-#define isb() __asm__ __volatile__ ("isb" : : : "memory") asm/barrier.h:#define dsb() __asm__ __volatile__ ("dsb" : : : "memory") asm/barrier.h-#define dmb() __asm__ __volatile__ ("dmb" : : : "memory") asm/barrier.h-#elif defined(CONFIG_CPU_XSC3) || __LINUX_ARM_ARCH__ == 6 asm/barrier.h-#define isb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h-#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \

_____________________________________
Can you try just adding a dsb() instead of the dummy read?

Quoted 1 line

If this also fixes your problem, it seems better to fix our writel macro

Not sure I understand who you are answering to exactly, as Fabio indicates his problem is solved.

Besides, Marek and I had in fact investigated barriers, adding some as I did in times past in mvgbe.c, and fiddling with the one already in dcache_flush_range(). None of this had any effect.

However, if our barriers are lacking, then they may fail us somehow on other occasions, so best is if we analyze the failings. Can you clarify in what respect exactly they are lacking? For instance, are all our barriers lacking, or only some, and which ones?

Amicalement,

Quoted 82 lines · Albert ARIBAUD

Hi Troy,

Quoted 77 lines · Troy Kisky

Quoted 14 lines · Fabio Estevam

Quoted 5 lines · Marek Vasut

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Quoted 9 lines

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Perhaps this because our memory barriers are lacking.

Linux has this code
asm/io.h:#define writel(v,c) ({ __iowmb();
writel_relaxed(v,c); })

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/barrier.h-#elif defined(CONFIG_ARM_DMA_MEM_BUFFERABLE) || defined(CONFIG_SMP)
asm/barrier.h-#define mb() do { dsb(); outer_sync(); }
while (0)
asm/barrier.h-#define rmb() dsb()
asm/barrier.h:#define wmb() mb()
asm/barrier.h-#else

asm/barrier.h-#define mb()              barrier()
asm/barrier.h-#define rmb()             barrier()
asm/barrier.h:#define wmb()             barrier()

asm/barrier.h-#endif

asm/barrier.h-#if __LINUX_ARM_ARCH__ >= 7 asm/barrier.h-#define isb() __asm__ __volatile__ ("isb" : : : "memory") asm/barrier.h:#define dsb() __asm__ __volatile__ ("dsb" : : : "memory") asm/barrier.h-#define dmb() __asm__ __volatile__ ("dmb" : : : "memory") asm/barrier.h-#elif defined(CONFIG_CPU_XSC3) || __LINUX_ARM_ARCH__ == 6 asm/barrier.h-#define isb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h-#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \

_____________________________________
Can you try just adding a dsb() instead of the dummy read?

Quoted 8 lines

Quoted 2 lines

If this also fixes your problem, it seems better to fix our writel macro

Not sure I understand who you are answering to exactly, as Fabio indicates his problem is solved.

Besides, Marek and I had in fact investigated barriers, adding some as I did in times past in mvgbe.c, and fiddling with the one already in dcache_flush_range(). None of this had any effect.

You tried adding a dsb() to dcache_flush_range()? That should have fixed the problem as well.

Quoted 7 lines

However, if our barriers are lacking, then they may fail us somehow on other occasions, so best is if we analyze the failings. Can you clarify in what respect exactly they are lacking? For instance, are all our barriers lacking, or only some, and which ones?

Amicalement,

Linux has

Kconfig:config ARM_DMA_MEM_BUFFERABLE
Kconfig- bool "Use non-cacheable memory for DMA" if (CPU_V6 ||
CPU_V6K) && !CPU_V7
Kconfig- depends on !(MACH_REALVIEW_PB1176 ||
REALVIEW_EB_ARM11MP || \
Kconfig- MACH_REALVIEW_PB11MP)
Kconfig- default y if CPU_V6 || CPU_V6K || CPU_V7
Kconfig- help

So, if this symbol is y then all writel/readl will be preceded by a dsb() as shown above.

However, u-boot probably uses cacheable memory for dma, so maybe u-boot is also correct with

asm/io.h-#define dmb() __asm__ __volatile__ ("" : : : "memory")
asm/io.h-#define __iormb() dmb()
asm/io.h:#define __iowmb() dmb()

and no dsb(), but perhaps flush_dcache still needs one at the end.

Troy

Quoted 126 lines · Troy Kisky

Quoted 82 lines · Albert ARIBAUD

Hi Troy,

Quoted 77 lines · Troy Kisky

Quoted 14 lines · Fabio Estevam

Quoted 5 lines · Marek Vasut

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

Quoted 8 lines

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

Quoted 9 lines

Excellent, managed to transfer 90MB via TFTP on mx28evk without a single timeout.

Tested-by Fabio Estevam <[email protected]>

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Perhaps this because our memory barriers are lacking.

Linux has this code
asm/io.h:#define writel(v,c) ({ __iowmb();
writel_relaxed(v,c); })

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/io.h-#ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
asm/io.h-#include <asm/barrier.h>
asm/io.h-#define __iormb() rmb()
asm/io.h:#define __iowmb() wmb()
asm/io.h-#else
asm/io.h-#define __iormb() do { } while (0)
asm/io.h:#define __iowmb() do { } while (0)
asm/io.h-#endif

asm/barrier.h-#elif defined(CONFIG_ARM_DMA_MEM_BUFFERABLE) || defined(CONFIG_SMP)
asm/barrier.h-#define mb() do { dsb(); outer_sync(); }
while (0)
asm/barrier.h-#define rmb() dsb()
asm/barrier.h:#define wmb() mb()
asm/barrier.h-#else

asm/barrier.h-#define mb()              barrier()
asm/barrier.h-#define rmb()             barrier()
asm/barrier.h:#define wmb()             barrier()

asm/barrier.h-#endif

asm/barrier.h-#if __LINUX_ARM_ARCH__ >= 7 asm/barrier.h-#define isb() __asm__ __volatile__ ("isb" : : : "memory") asm/barrier.h:#define dsb() __asm__ __volatile__ ("dsb" : : : "memory") asm/barrier.h-#define dmb() __asm__ __volatile__ ("dmb" : : : "memory") asm/barrier.h-#elif defined(CONFIG_CPU_XSC3) || __LINUX_ARM_ARCH__ == 6 asm/barrier.h-#define isb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
asm/barrier.h- : : "r" (0) : "memory")
asm/barrier.h-#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \

_____________________________________
Can you try just adding a dsb() instead of the dummy read?

Quoted 8 lines

Quoted 2 lines

If this also fixes your problem, it seems better to fix our writel macro

Not sure I understand who you are answering to exactly, as Fabio indicates his problem is solved.

Besides, Marek and I had in fact investigated barriers, adding some as I did in times past in mvgbe.c, and fiddling with the one already in dcache_flush_range(). None of this had any effect.

You tried adding a dsb() to dcache_flush_range()? That should have fixed the problem as well.

Quoted 7 lines

However, if our barriers are lacking, then they may fail us somehow on other occasions, so best is if we analyze the failings. Can you clarify in what respect exactly they are lacking? For instance, are all our barriers lacking, or only some, and which ones?

Amicalement,

Linux has

Kconfig:config ARM_DMA_MEM_BUFFERABLE
Kconfig- bool "Use non-cacheable memory for DMA" if (CPU_V6 ||
CPU_V6K) && !CPU_V7
Kconfig- depends on !(MACH_REALVIEW_PB1176 ||
REALVIEW_EB_ARM11MP || \
Kconfig- MACH_REALVIEW_PB11MP)
Kconfig- default y if CPU_V6 || CPU_V6K || CPU_V7
Kconfig- help

So, if this symbol is y then all writel/readl will be preceded by a dsb() as shown above.

However, u-boot probably uses cacheable memory for dma, so maybe u-boot is also correct with

asm/io.h-#define dmb() __asm__ __volatile__ ("" : : : "memory")
asm/io.h-#define __iormb() dmb()
asm/io.h:#define __iowmb() dmb()

Quoted 5 lines

and no dsb(), but perhaps flush_dcache still needs one at the end.

Troy

for armv7, flush dcache does have a dsb.

//u-boot
#define CP15DSB asm volatile ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0))

//linux
asm/barrier.h:#define dsb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0) : "memory")

Don't know why I didn't see that before.
So, I don't know why that wasn't good enough.

Maybe CONFIG_SYS_DCACHE_OFF was set and

void flush_dcache_range(unsigned long start, unsigned long stop) {
}

Needs a dsb too???

Troy

Quoted 3 lines

Besides, Marek and I had in fact investigated barriers, adding some as I did in times past in mvgbe.c, and fiddling with the one already in dcache_flush_range(). None of this had any effect.

You tried adding a dsb() to dcache_flush_range()? That should have fixed the problem as well.

Quoted 6 lines

However, if our barriers are lacking, then they may fail us somehow on other occasions, so best is if we analyze the failings. Can you clarify in what respect exactly they are lacking? For instance, are all our barriers lacking, or only some, and which ones?

Amicalement,

Linux has

Kconfig:config ARM_DMA_MEM_BUFFERABLE
Kconfig- bool "Use non-cacheable memory for DMA" if (CPU_V6 ||
CPU_V6K) && !CPU_V7
Kconfig- depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP

|| \
Kconfig-                     MACH_REALVIEW_PB11MP)
Kconfig-        default y if CPU_V6 || CPU_V6K || CPU_V7
Kconfig-        help

So, if this symbol is y then all writel/readl will be preceded by a dsb() as shown above.

However, u-boot probably uses cacheable memory for dma, so maybe u-boot is also correct with

asm/io.h-#define dmb() __asm__ __volatile__ ("" : : : "memory")
asm/io.h-#define __iormb() dmb()
asm/io.h:#define __iowmb() dmb()

and no dsb(), but perhaps flush_dcache still needs one at the end.

Troy

Hi Troy,

Quoted 3 lines · Troy Kisky

Quoted 3 lines

Besides, Marek and I had in fact investigated barriers, adding some as I did in times past in mvgbe.c, and fiddling with the one already in dcache_flush_range(). None of this had any effect.

Quoted 2 lines

You tried adding a dsb() to dcache_flush_range()? That should have fixed the problem as well.

There already was a memory barrier -- the only one kind known bu ARM926J-S, which is a write buffer(s) drain -- and no, it should not have fixed the problem (and did not), because the issue is not about pushing the transactions out of the CPU soon enough; it is about having the EMI perform them before it passes our 'go' command to the ENET DMA.

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

---
 drivers/net/fec_mxc.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

drivers/net/fec_mxc.c +15 -0

index 97bf8fe..ec5b9db 100644
@@ -737,6 +737,28 @@ static int fec_send(struct eth_device *dev, void *packet, int length)
 	flush_dcache_range(addr, addr + size);
 
 	/*
+	 * Below we read the DMA descriptor's last four bytes back from the
+	 * DRAM. This is important in order to make sure that all WRITE
+	 * operations on the bus that were triggered by previous cache FLUSH
+	 * have completed.
+	 *
+	 * Otherwise, on MX28, it is possible to observe a corruption of the
+	 * DMA descriptors. Please refer to schematic "Figure 1-2" in MX28RM
+	 * for the bus structure of MX28. The scenario is as follows:
+	 *
+	 * 1) ARM core triggers a series of WRITEs on the AHB_ARB2 bus going
+	 *    to DRAM due to flush_dcache_range()
+	 * 2) ARM core writes the FEC registers via AHB_ARB2
+	 * 3) FEC DMA starts reading/writing from/to DRAM via AHB_ARB3
+	 *
+	 * Note that 2) does sometimes finish before 1) due to reordering of

except it tends to minimize Marek's own contribution to the fix, which is by far the most important.

Afterthought:

On Fri, 12 Jul 2013 07:57:18 +0200, Albert ARIBAUD

Quoted 2 lines · <[email protected]> wrote:

except it tends to minimize Marek's own contribution to the fix, which is by far the most important.

'The most important' 'by far' being of course Marek's contribution, not minimizing it. :)

This being a bugfix patch, and having been tested twice, I suggest that it go in 2013.07, maybe with the commit message reduced to its first paragraph above -- although of course I do appreciate the second one, except it tends to minimize Marek's own contribution to the fix, which is by far the most important.

The MX28 multi-layer AHB bus can be too slow and trigger the FEC DMA too early, before all the data hit the DRAM. This patch ensures the data are written in the RAM before the DMA starts. Please see the comment in the patch for full details.

This patch was produced with an amazing help from Albert Aribaud, who pointed out it can possibly be such a bus synchronisation issue.

Signed-off-by Marek Vasut <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Stefano Babic <[email protected]>

---