Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662 was reviewed by Christian Mauderer
-- Christian Mauderer started a new discussion on bsps/aarch64/raspberrypi/include/bsp/raspberrypi-dma.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662#note_129007 > +} rpi_dma_channel; > + > +void* rpi_unaligned_mem_to_mem_dma( rpi_dma_channel channel, void *src, > uint32_t length ); Please add a short doxygen description for all public functions. Only a short description what they should do and what the parameters are. Two or three sentences is usually quite useful already. For example for this function it could be: ``` Copies @a length bytes of data from @a src to some randomly allocated location. Use the DMA @a channel for that. @returns the random location. It's the callers responsibility to free the returned location. ``` PS: Regarding the destination of this special function: Please also take a look at the comment from Kinseys in the C file. It's not really useful for a user to copy a memory to some random location. -- Christian Mauderer started a new discussion on bsps/aarch64/raspberrypi/include/bsp/raspberrypi-dma.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662#note_129008 > + * @file > + * > + * @ingroup raspberrypi_4_dma Where do you define that Doxygen group? The Doxygen guide links to two examples: https://docs.rtems.org/docs/main/eng/coding-doxygen.html#header-file-examples I would suggest to use the same method like in this file: [https://gitlab.rtems.org/rtems/rtos/rtems/-/blob/main/cpukit/include/rtems/score/thread.h](https://gitlab.rtems.org/rtems/rtos/rtems/-/blob/main/cpukit/include/rtems/score/thread.h#L79) Your `ingroup` would be some raspberrypi group. Check the other files in the BSP for that. -- Christian Mauderer started a new discussion on bsps/aarch64/raspberrypi/dma/raspberrypi-dma.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662#note_129009 > + > +#include <bsp/raspberrypi-dma.h> > +#define CB_ALIGNMENT 32 There is already an `CPU_CACHE_LINE_BYTES` define for that in RTEMS. Please use that. -- Christian Mauderer started a new discussion on bsps/aarch64/raspberrypi/dma/raspberrypi-dma.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662#note_129010 > +#define CB_ALIGNMENT 32 > +#define SIZE 16 > +#define BUFFER_SIZE 64 `SIZE` and `BUFFER_SIZE` seem to be unused. -- Christian Mauderer commented on a discussion on bsps/aarch64/raspberrypi/dma/raspberrypi-dma.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662#note_129011 > + > + > +void *rpi_unaligned_mem_to_mem_dma( rpi_dma_channel channel, void *src, > uint32_t length ) I agree with Kinsey. Either you should remove the function or you should give the user the possibility to pass a destination. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/662 You're receiving this email because of your account on gitlab.rtems.org.
_______________________________________________ bugs mailing list [email protected] http://lists.rtems.org/mailman/listinfo/bugs
