Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120 
was reviewed by Gedare Bloom

--
  
Gedare Bloom started a new discussion on 
bsps/aarch64/raspberrypi/include/bsp/watchdog.h: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120#note_109475

 > +#endif
 > +
 > +struct rpi_wdt_timeout

use the consistent naming convention for this bsp. I think it is either 
`raspberrypi` or `raspberrypi_4` (although I think that should have been 
`raspberrypi4`.

--
  
Gedare Bloom started a new discussion on 
bsps/aarch64/raspberrypi/include/bsp/watchdog.h: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120#note_109476

 > +#endif
 > +
 > +struct rpi_wdt_timeout

spell out `watchdog`.

--
  
Gedare Bloom started a new discussion on 
bsps/aarch64/raspberrypi/include/bsp/watchdog.h: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120#note_109477

 > + * @param timeout timeout value.
 > + */
 > +void rpi_wdt_init(int timeout);

The interface for this could probably be copied from 
`sparc/leon3/include/bsp/watchdog.h`. Although there is no standard, it is a 
good idea to try to be close to similar interfaces in other BSPs when it makes 
sense.

--
  
Gedare Bloom started a new discussion on 
bsps/aarch64/raspberrypi/start/watchdog.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120#note_109478

 > +  volatile uint32_t raspberrypi_PM_WDOG = BCM2835_REG(BCM2711_PM_WDOG);
 > +  volatile uint32_t raspberrypi_PM_RSTC = BCM2835_REG(BCM2711_PM_RSTC);
 > +  raspberrypi_PM_WDOG = BCM2711_PM_PASSWD_MAGIC | ((rpi_wdt_timeout.timeout 
 > << 16) & BCM2711_PM_WDOG_MASK);

line length

--
  
Gedare Bloom started a new discussion on 
bsps/aarch64/raspberrypi/start/watchdog.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120#note_109479

 > +void rpi_wdt_refresh()
 > +{
 > +  volatile uint32_t raspberrypi_PM_WDOG = BCM2835_REG(BCM2711_PM_WDOG);

maybe add some static helper functions in this file to read/write the WDOG and 
RSTC registers. it will make this more readable.

--
  
Gedare Bloom started a new discussion on 
bsps/aarch64/raspberrypi/start/watchdog.c: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120#note_109480

 > +#include <bsp/watchdog.h>
 > +
 > +static struct rpi_wdt_timeout rpi_wdt_timeout;

this doesn't seem like it needs to be a global variable. It appears only to be 
used where it gets passed directly as an argument?




-- 
View it on GitLab: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/120
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

Reply via email to