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

[yangn0 at qq.com]

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<mailto: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<mailto: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/5e54b37b/attachment.htm>