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

[Kinsey Moore]

Hi Ning,
Patch 1 looks good. Instead of renaming it, you can include bcm2711.h in
the RPi4's raspberrypi.h or in bsp.h so that the RPi4 BSP remains largely
unchanged.

Kinsey

On Wed, Apr 10, 2024 at 2:14 AM yangn0 at qq.com <yangn0 at qq.com> wrote:

> Hi Mr. Kinsey Moore,
>
> Thank you very much for your help.
>
> I just sent patch 1. And about patch2, I have a question:
>
> In shared/dev/clock/bcm2835-system-timer.c:
>
> #if RTEMS_BSP == raspberrypi4b
> #include <bsp/bcm2711.h>
> #else
> #include <bsp/raspberrypi.h>
> #endif /* RTEMS_BSP */
>
> Do I need to rename bcm2711.h in rpi4 BSP to raspberrypi.h, this will
> modify many files in rpi4 BSP.
>
> Best regards,
>
> Ning
> ------------------------------
> *From:* Kinsey Moore <kinsey.moore at oarcorp.com>
> *Sent:* Tuesday, April 9, 2024 11:10
> *To:* Ning Yang <yangn0 at qq.com>
> *Cc:* devel at rtems.org <devel at rtems.org>
> *Subject:* Re: [PATCH 1/2] dev/clock: Move bcm2835-system-timer driver to
> shared space
>
> 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
>
> _______________________________________________
> 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/20240410/121a4e41/attachment.htm>