fpga: zynqpl: Fixed bug in alignment routine
Subsystems (2)
PATCH 1/1 fpga: zynqpl: Fixed bug in alignment routine
The aligned buffer is always with a higher address, so copying should run from the end of the buffer to the beginning, and not the other way around.
--- drivers/fpga/zynqpl.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/fpga/zynqpl.c +3 -2
@@ -173,7 +173,8 @@ int zynq_load(Xilinx_desc *desc, const void *buf, size_t bsize) {unsigned long ts;/* Timestamp */ u32 partialbit =0 ; -u32 i, control, isr_status, status, swap, diff; +u32 control, isr_status, status, swap, diff; +int i;u32 *buf_start;/* Detect if we are going working with partial or full bitstream */
@@ -206,7 +207,7 @@ int zynq_load(Xilinx_desc *desc, const void *buf, size_t bsize)printf ("%s: Align buffer at %x to %x(swap %d)\n" , __func__, (u32 )buf_start, (u32 )new_buf, swap); -for (i =0 ; i < (bsize/4 ); i++) +for (i = (bsize/4 )-1 ; i >=0 ; i--) new_buf[i] =load_word (&buf_start[i], swap); swap =SWAP_DONE ;
Signature · 1 line
1.7.2.3
PATCH 1/1 fpga: zynqpl: Fixed bug in alignment routine
Hi Eli,
Quoted 24 lines · Eli Billauer
The aligned buffer is always with a higher address, so copying should run from the end of the buffer to the beginning, and not the other way around.
Signed-off-by Eli Billauer <[email protected]>--- drivers/fpga/zynqpl.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)drivers/fpga/zynqpl.c +2
-1@@ -173,7 +173,8 @@ int zynq_load(Xilinx_desc *desc, const void *buf, size_t bsize) {unsigned long ts;/* Timestamp */ u32 partialbit =0 ; -u32 i, control, isr_status, status, swap, diff; +u32 control, isr_status, status, swap, diff; +int i;u32 *buf_start;/* Detect if we are going working with partial or full bitstream */
drivers/fpga/zynqpl.c int zynq_load(Xilinx_desc *desc, const void *buf, · line 213
Quoted 8 lines
drivers/fpga/zynqpl.c +1
-1@@ -206,7 +207,7 @@ int zynq_load(Xilinx_desc *desc, const void *buf, size_t bsize) printf("%s: Align buffer at %x to %x(swap %d)\n", __func__, (u32)buf_start, (u32)new_buf, swap); -for (i =0 ; i < (bsize/4 ); i++) +for (i = (bsize/4 )-1 ; i >=0 ; i--) new_buf[i] =load_word (&buf_start[i], swap);
This looks like not correct because if you look at the code above this, it always ensuring that the new aligned buffer start is in front of the actual buffer. That is for example if actual buff start is at 0x6C, then it provides new buf aligned at 0x00 and copying word by word from 0x6C to 0x00.
But here if you do copy word by word from the end, it will end up in
corrupting the actual data.(For example if our buff len is some 0x100 the you are trying to copy from 0x16c to 0x100 which will corrupt the actual data at 0x100).
Regards,
DP
Quoted 10 lines
swap = SWAP_DONE;--
1.7.2.3_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 1/1 fpga: zynqpl: Fixed bug in alignment routine
Quoted 3 lines · S Durga Prasad Paladugu
This looks like not correct because if you look at the code above this, it always ensuring that the new aligned buffer start is in front of the actual buffer
Maybe it should, but it doesn't. In the boot log it says:
zynq_load: Align buffer at 10006f to 100080(swap 1)
In fact, it makes sense to align upwards. Copying the buffer downwards would destroy the data in the beginning of the buffer. Not that it probably matters either way.
Regards,
Eli
PATCH 1/1 fpga: zynqpl: Fixed bug in alignment routine
Quoted 7 lines · Eli Billauer
Quoted 1 line · S Durga Prasad Paladugu
This looks like not correct because if you look at the code above this, it always ensuring that the new aligned buffer start is in front of the actual buffer
Maybe it should, but it doesn't. In the boot log it says:
zynq_load: Align buffer at 10006f to 100080(swap 1)
I have checked this and there is bug in code.
I have tested it by:
+ int i;
+
+ for (i = 0; i < 100; i++) {
+ printf("%x: align %x\n", i, ALIGN((u32)i, ARCH_DMA_MINALIGN));
+ }
Does this work for you? If yes, I will send this as regular patch.
drivers/fpga/zynqpl.c +4 -3
@@ -289,10 +289,11 @@ static int zynq_dma_xfer_init(u32 partialbit)static u32 *zynq_align_dma_buffer (u32 *buf,u32 len,u32 swap) {u32 *new_buf; -u32 i; +u32 i, align; -if ((u32 )buf !=ALIGN ((u32 )buf,ARCH_DMA_MINALIGN )) { - new_buf = (u32 *)ALIGN ((u32 )buf,ARCH_DMA_MINALIGN ); + align =ALIGN ((u32)(buf -ARCH_DMA_MINALIGN +1 ),ARCH_DMA_MINALIGN ); +if ((u32 )buf != align) { + new_buf = (u32 *)align;/* * This might be dangerous but permits to flash if
Thanks,
Michal
Signature · 14 lines
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 263 bytes Desc: OpenPGP digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140318/d420533a/attachment.pgp>
PATCH 1/1 fpga: zynqpl: Fixed bug in alignment routine
Hello,
We're completely out of sync with our git repositories, but I changed the relevant parts in my zynqpl.c (with the patch of this thread not applied), and it worked well. The alignment went downwards as expected.
Regards,
Eli
Quoted 20 lines · Michal Simek
Does this work for you? If yes, I will send this as regular patch.
drivers/fpga/zynqpl.c +4
-3@@ -289,10 +289,11 @@ static int zynq_dma_xfer_init(u32 partialbit)static u32 *zynq_align_dma_buffer (u32 *buf,u32 len,u32 swap) {u32 *new_buf; -u32 i; +u32 i, align; -if ((u32 )buf !=ALIGN ((u32 )buf,ARCH_DMA_MINALIGN )) { - new_buf = (u32 *)ALIGN ((u32 )buf,ARCH_DMA_MINALIGN ); + align =ALIGN ((u32)(buf -ARCH_DMA_MINALIGN +1 ),ARCH_DMA_MINALIGN ); +if ((u32 )buf != align) { + new_buf = (u32 *)align;
PATCH 1/1 fpga: zynqpl: Fixed bug in alignment routine
Hi,
Quoted 3 lines · Eli Billauer
Hello,
We're completely out of sync with our git repositories,
but I changed the relevant parts in my zynqpl.c (with the patch of this thread not applied), and it worked well. The alignment went downwards as expected.
That's interesting. Anyway we will look at my proposed patch because from my test from yesterday ALIGN is ALIGN_UP but we need ALIGN_DOWN.
Thanks,
Michal
Signature · 14 lines
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 263 bytes Desc: OpenPGP digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140319/cab20cce/attachment.pgp>