This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Bug fix in cl_8900a ethernet driver


Laurent Gonzalez wrote:
hi folks,

the file attached contains a patch to fix this bug.

In cs8900a_send, the cast from data to sdata is not safe. If data is not
aligned on a short boundary (even address), the 16bit read with the
pointer sdata, may return an unpredictible value or raise an alignement
error (depending the hardware).
The bug will happen if the variable odd_byte becomes true, and the next
packet has an aligned value for data. the first byte is joined with
saved_data, and the pointer data becomes odd.

Thanks for spotting this. This indeed looks like a problem, although I'm slightly concerned that the patch is endian-specific.... but looking at the driver, it looks to be endian-specific in its treatment of odd_byte too. Nevertheless, I thought I may as well fix it just in case since even if the CS8900 is only ever used with ARMs which it may be, ARMs have been known to be big-endian.


However it's somewhat sub-optimal to _always_ resort to byte fiddling. Most times the amount of data to be spent will be large enough that optimising the aligned case is worth it. So I've created the attached patch. Since I don't have cs8900 hardware, please let me know if it works for you and I'll check it in!

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine
Index: src/if_cs8900a.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/eth/cl/cs8900a/current/src/if_cs8900a.c,v
retrieving revision 1.6
diff -u -5 -p -r1.6 if_cs8900a.c
--- src/if_cs8900a.c	14 Jun 2002 22:01:17 -0000	1.6
+++ src/if_cs8900a.c	11 Apr 2003 02:56:23 -0000
@@ -449,24 +449,42 @@ cs8900a_send(struct eth_drv_sc *sc, stru
         len = sg_list[i].len;
 
         if (len > 0) {
             /* Finish the last word. */
             if (odd_byte) {
+#if CYG_BYTEORDER == CYG_MSBFIRST
+                saved_data = ((cyg_uint16)*data++) | (saved_data << 8);
+#else
                 saved_data |= ((cyg_uint16)*data++) << 8;
+#endif
                 HAL_WRITE_UINT16(cpd->base+CS8900A_RTDATA, saved_data);
                 len--;
                 odd_byte = false;
             }
-            /* Output contiguous words. */
-            sdata = (cyg_uint16 *)data;
-            while (len > 1) {
-                HAL_WRITE_UINT16(cpd->base+CS8900A_RTDATA, *sdata++);
-                len -= sizeof(cyg_uint16);
+            if ((CYG_ADDRESS)data & 0x1 == 0) {
+                /* Aligned on 16-bit boundary, so output contiguous words. */
+                sdata = (cyg_uint16 *)data;
+                while (len > 1) {
+                    HAL_WRITE_UINT16(cpd->base+CS8900A_RTDATA, *sdata++);                    
+                    len -= sizeof(cyg_uint16);
+                }
+                data = (cyg_uint8 *)sdata;
+            } else {
+                /* Not 16-bit aligned, so byte copy */
+                while (len > 1) {
+                    saved_data = (cyg_uint16)*data++;   // reuse saved_data
+#if CYG_BYTEORDER == CYG_MSBFIRST
+                    saved_data =  ((cyg_uint16)*data++) | (saved_data << 8);
+#else
+                    saved_data |= ((cyg_uint16)*data++) << 8;
+#endif
+                    HAL_WRITE_UINT16(cpd->base+CS8900A_RTDATA, saved_data);
+                    len -= sizeof(cyg_uint16);
+                }
             }
             /* Save last byte, if necessary. */
             if (len == 1) {
-                data = (cyg_uint8 *)sdata;
                 saved_data = (cyg_uint16)*data;
                 odd_byte = true;
             }
         }
     }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]