Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47 was reviewed by Gedare Bloom
-- Gedare Bloom started a new discussion on bsps/include/dev/serial/arm-pl011.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109464 > +#define RXFIFO_IRQ_TRIGGER_LEVEL FIFO_LEVEL_ONE_HALF > + > +enum fifo_trigger_level { This type should be prefaced with the scope, like `arm_pl011_fifo_trigger_level`. That said, it appears to be unused and can be removed? the `ONE_EIGHTH` and `ONE_HALF` are used, you can just directly `#define` those to `(0)` and `(2)`.? -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109465 > +} > + > +static inline void arm_pl011_write_char(volatile pl011_base *regs_base, > const char ch) line length -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109466 > + > + /* Find the fractional part */ > + const uint16_t scalar = 1 << (PL011_UARTFBRD_BAUD_DIVFRAC_WIDTH + 1); declare all the variables at the start of the block. -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109467 > + > + uint32_t ibrd, fbrd; > + if (arm_pl011_compute_baudrate_params(&ibrd, &fbrd, baud, context->clock, > 3) != 0) line length. Better to do like `err = arm_...; if (err != 0)` -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109468 > +} > + > +static bool arm_pl011_set_attributes( this helper function is pretty long. consider refactoring to add more helpers -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109469 > - arm_pl011_context *ctx = (arm_pl011_context *) base; > +#ifndef BSP_CONSOLE_USE_INTERRUPTS > + const why only `const` with interrupts? -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109470 > + regs->uartlcr_h |= PL011_UARTLCR_H_FEN; > + arm_pl011_disable_irq(regs, PL011_UARTI_MASK); > + const rtems_status_code sc = rtems_interrupt_handler_install( declare variables at start. these variables can be decld above in an `#ifdef`, or you might refactor this entire block of code within this `#ifdef` to a helper function -- Gedare Bloom started a new discussion on bsps/shared/dev/serial/arm-pl011.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47#note_109471 > +{ > +#ifdef BSP_CONSOLE_USE_INTERRUPTS > + arm_pl011_context *context = (arm_pl011_context *) base; this block could also be `write_support_interrupt()` -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/47 You're receiving this email because of your account on gitlab.rtems.org.
_______________________________________________ bugs mailing list bugs@rtems.org http://lists.rtems.org/mailman/listinfo/bugs
