Hi Michael, understood … I changed the fix according to you proposal and tested it … works fine,too:
https://github.com/ptka/incubator-nuttx/commit/09be64bb1f2fff9f85a392e0fab6915f9083fe2d <https://github.com/ptka/incubator-nuttx/commit/09be64bb1f2fff9f85a392e0fab6915f9083fe2d> struct cdcecm_driver_s { /* USB CDC-ECM device */ struct usbdevclass_driver_s usbdev; /* USB device class vtable */ struct usbdev_devinfo_s devinfo; FAR struct usbdev_req_s *ctrlreq; /* Allocated control request */ FAR struct usbdev_ep_s *epint; /* Interrupt IN endpoint */ FAR struct usbdev_ep_s *epbulkin; /* Bulk IN endpoint */ FAR struct usbdev_ep_s *epbulkout; /* Bulk OUT endpoint */ uint8_t config; /* Selected configuration number */ aligned_data(2) uint8_t pktbuf[CONFIG_NET_ETH_PKTSIZE + CONFIG_NET_GUARDSIZE]; struct usbdev_req_s *rdreq; /* Single read request */ /Piet > Am 17.01.2022 um 10:42 schrieb Michael Jung <mij...@gmx.net>: > > Hi Peter, > > good finding! > > There is actually a comment in this regard in include/nuttx/net/netdev.h: >> The d_buf array must be aligned to two-byte, 16-bit address boundaries in > order to support aligned 16-bit accesses performed by the network. > > W.r.t. to your proposed fix I think it would be more appropriate to make > the alignment requirement explicit by using the 'aligned_data' macro from > include/nuttx/compiler.h. > > Thanks! > Michael > > Am Mo., 17. Jan. 2022 um 09:01 Uhr schrieb Peter Kalbus > <p...@mailbox.org.invalid <mailto:p...@mailbox.org.invalid>>: > >> Hi all, >> >> during the bring-up of the composite-cdcecm a alignment issue result in >> hard faults (always on the used rp2040 based device). >> >> ANALYSIS: >> >> The root cause seems to be the way the pktbuf is defined in >> cdcecm_driver_s (driver/usbdev/cdcecm.c): >> >> struct cdcecm_driver_s >> { >> /* USB CDC-ECM device */ >> >> struct usbdevclass_driver_s usbdev; /* USB device class vtable */ >> struct usbdev_devinfo_s devinfo; >> FAR struct usbdev_req_s *ctrlreq; /* Allocated control request */ >> FAR struct usbdev_ep_s *epint; /* Interrupt IN endpoint */ >> FAR struct usbdev_ep_s *epbulkin; /* Bulk IN endpoint */ >> FAR struct usbdev_ep_s *epbulkout; /* Bulk OUT endpoint */ >> uint8_t config; /* Selected configuration >> number */ >> >> uint8_t pktbuf[CONFIG_NET_ETH_PKTSIZE + >> CONFIG_NET_GUARDSIZE]; >> >> >> Due to 'config‘ element ‚ pktbuf‘ in the structure is not aligned in >> anyway from the struct itself. >> >> The driver structure is allocation during driver initialization in >> cdcecm_classobject(): >> >> /* Initialize the driver structure */ >> >> self = kmm_zalloc(sizeof(struct cdcecm_driver_s)); >> if (!self) >> { >> nerr("Out of memory!\n"); >> return -ENOMEM; >> } >> >> /* Network device initialization */ >> >> self->dev.d_buf = self->pktbuf; >> >> As ’ self’ points to an aligned address, 'pktbuf' is always aligned at an >> odd address. >> >> Once the driver is initialized and processes received data in >> cdcecm_receive() the driver creates a hard fault at: >> >> #ifdef CONFIG_NET_IPv4 >> if (BUF->type == HTONS(ETHTYPE_IP)) >> { >> >> BUF/eth_hdr_s are defined as: >> >> #define BUF ((struct eth_hdr_s *)self->dev.d_buf) >> >> >> struct eth_hdr_s >> { >> uint8_t dest[6]; /* Ethernet destination address (6 bytes) */ >> uint8_t src[6]; /* Ethernet source address (6 bytes) */ >> uint16_t type; /* Type code (2 bytes) */ >> }; >> >> >> Element ’ type’ is always odd aligned and access to it results in all >> cases to a hard fault on the used rp2040 based board. >> >> PROPOSAL: >> >> Assuming the analysis is correct, there could be different options to fix >> there issue. >> Proposal would be to let the compiler do the job and not introduce any >> special code or elements in the structure to fix it programmatically. >> >> Analysing cdcdcm_driver_s in more details: >> >> FAR struct usbdev_ep_s *epbulkout; /* Bulk OUT endpoint */ >> uint8_t config; /* Selected configuration >> number */ >> >> uint8_t pktbuf[CONFIG_NET_ETH_PKTSIZE + >> CONFIG_NET_GUARDSIZE]; >> >> >> Element 'epbulkout' is a 32bit pointer and thus always aligned to 32bit. >> Element 'config‘ following is also aligned to 32bit —> as it’s only an >> uint8_t is doesn’t need to be aligned at all. >> Element ‚pktbuf‘ is odd aligned, but needs to be at least 16bit aligned. >> >> Exchanging 'config‘ and 'pktbuf‘ in the structure solves the issue: >> >> FAR struct usbdev_ep_s *epbulkout; /* Bulk OUT endpoint */ >> >> uint8_t pktbuf[CONFIG_NET_ETH_PKTSIZE + >> CONFIG_NET_GUARDSIZE]; >> >> uint8_t config; /* Selected configuration >> number */ >> >> >> Element ‚pktbuf‘ is now 32bit aligned as it’s following a 32bit pointer. >> >> The change has been tested on an rp2040 based device. >> >> The fix can be reviewed at: >> https://github.com/ptka/incubator-nuttx/commit/01732850ad345b5e2a9af24f788935d8f7928781 >> < >> https://github.com/ptka/incubator-nuttx/commit/01732850ad345b5e2a9af24f788935d8f7928781 >> >> <https://github.com/ptka/incubator-nuttx/commit/01732850ad345b5e2a9af24f788935d8f7928781> >>> >> >> Please comment on the analysis and proposed fix. >> >> /Piet
