[PATCH 1/2] dev/clock: Move bcm2835-system-timer driver to shared space

[Kinsey Moore]

One comment inline below.

On Tue, Apr 9, 2024 at 10:02 AM Ning Yang <yangn0 at qq.com> wrote:

> This patch moves the bcm2835 system timer driver in the arm/raspberrypi
> directory to the shared directory.
>
> Made some changes in the include section to adapt to rpi4 BSP.
> ---
>  .../clockdrv.c => shared/dev/clock/bcm2835-system-timer.c} | 7 +++++++
>  spec/build/bsps/arm/raspberrypi/obj.yml                    | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>  rename bsps/{arm/raspberrypi/clock/clockdrv.c =>
> shared/dev/clock/bcm2835-system-timer.c} (95%)
>
> diff --git a/bsps/arm/raspberrypi/clock/clockdrv.c
> b/bsps/shared/dev/clock/bcm2835-system-timer.c
> similarity index 95%
> rename from bsps/arm/raspberrypi/clock/clockdrv.c
> rename to bsps/shared/dev/clock/bcm2835-system-timer.c
> index 8d220d51ba..2725735860 100644
> --- a/bsps/arm/raspberrypi/clock/clockdrv.c
> +++ b/bsps/shared/dev/clock/bcm2835-system-timer.c
> @@ -22,7 +22,14 @@
>  #include <bsp.h>
>  #include <bsp/irq.h>
>  #include <bsp/irq-generic.h>
> +
> +#if RTEMS_BSP == raspberrypi4b
> +#include <bsp/bcm2711.h>
> +#define BCM2835_REG(addr)          *(volatile uint32_t*)(addr)
> +#else
>  #include <bsp/raspberrypi.h>
> +#endif /* RTEMS_BSP */
> +
>

This change should really be in patch 2, but there is another issue with
this change. This pattern is not used in existing code that I could find
because it's fragile and introduces BSP-specific code into a shared driver
that will need to be updated for every single BSP that eventually wants to
use it. Either the RPi4 BSP should define BCM2835_REG as a 32bit register
access for compatibility or this driver should have its register accesses
renamed to something more generic such as PI32_REG which the RPi3 and RPi4
BSPs can provide for usage with this driver.

 #include <rtems/timecounter.h>
>
>  /* This is defined in ../../../shared/dev/clock/clockimpl.h */
> diff --git a/spec/build/bsps/arm/raspberrypi/obj.yml
> b/spec/build/bsps/arm/raspberrypi/obj.yml
> index ec5c82a8fb..ea370829df 100644
> --- a/spec/build/bsps/arm/raspberrypi/obj.yml
> +++ b/spec/build/bsps/arm/raspberrypi/obj.yml
> @@ -26,7 +26,6 @@ install:
>    - bsps/arm/raspberrypi/include/bsp/vc.h
>  links: []
>  source:
> -- bsps/arm/raspberrypi/clock/clockdrv.c
>  - bsps/arm/raspberrypi/console/console-config.c
>  - bsps/arm/raspberrypi/console/fb.c
>  - bsps/arm/raspberrypi/console/fbcons.c
> @@ -47,6 +46,7 @@ source:
>  - bsps/arm/shared/cp15/arm-cp15-set-exception-handler.c
>  - bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
>  - bsps/arm/shared/start/bsp-start-memcpy.S
> +- bsps/shared/dev/clock/bcm2835-system-timer.c
>  - bsps/shared/dev/cpucounter/cpucounterfrequency.c
>  - bsps/shared/dev/cpucounter/cpucounterread.c
>  - bsps/shared/dev/getentropy/getentropy-cpucounter.c
> --
> 2.34.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20240409/8c8ed676/attachment-0001.htm>