Re: [PATCH 10/20] clocksource/drivers/vf-pit: Encapsulate the macros

From: Daniel Lezcano
Date: Mon Jul 28 2025 - 04:36:09 EST


On 07/07/2025 14:03, Ghennadi Procopciuc wrote:
On 7/5/2025 7:01 PM, Daniel Lezcano wrote:
Pass the base address to the macro, so we can use the macro with
multiple instances of the timer because we deal with different base
address. At the same time, change writes to the register to the
existing corresponding functions.

Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
---
drivers/clocksource/timer-vf-pit.c | 35 ++++++++++++++++--------------
1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 066d0d2600f4..9c5e06506c26 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -16,18 +16,21 @@
#define PITMCR 0x00
#define PIT0_OFFSET 0x100
#define PIT_CH(n) (PIT0_OFFSET + 0x10 * (n))
-#define PITLDVAL 0x00
+
#define PITCVAL 0x04
-#define PITTCTRL 0x08
-#define PITTFLG 0x0c

The registers PITLDVAL, PITCVAL, PITTCTRL, and PITTFLG refer to individual PIT channels rather than global PIT registers. Shouldn't this distinction be reflected in their naming? I would suggest prefixing them with PIT_CH_ to improve clarity and indicate their per-channel scope.

Reasonably, I would like to keep them as is otherwise it would be out of the scope of these changes which are aiming to set the scene for multiple PITs support. If the naming convention has to be changed, that could be addressed later in a separate series.

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog