Re: perf: Add Raspberry Pi BCM2835 AXI PMU driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `num_started` field in `struct rpi_axi_hw_events` is modified but never read or utilized.
--

--- Patch [1]: perf: Add Raspberry Pi BCM2835 AXI PMU driver ---
commit 0a35d2ad48de3bdbbbe637f581c580caecb30465
Author: Ian Rogers <[email protected]>
Subject: perf: Add Raspberry Pi BCM2835 AXI PMU driver

This commit adds a new performance monitoring driver for the Raspberry Pi AXI
bus (BCM2835/2711), exposing system-level and VideoCore PMU hardware to the
Linux perf subsystem.

> diff --git a/drivers/perf/rpi_axi_pmu.c b/drivers/perf/rpi_axi_pmu.c
> new file mode 100644
> index 0000000000000..d8efc8bb205fe
> --- /dev/null
> +++ b/drivers/perf/rpi_axi_pmu.c
> @@ -378,9 +378,9 @@ static bool config_is_valid(struct rpi_axi_pmu *pmu, __u64 config);
>  struct rpi_axi_hw_events {
>  	int monitored_bus[NUM_BUS_WATCHERS_PER_MONITOR];
>  	int filter[NUM_BUS_WATCHERS_PER_MONITOR];
>  	int refcount[NUM_BUS_WATCHERS_PER_MONITOR];
>  	int num_monitored;
>  	int num_started;

[Severity: Low]
Is this num_started field needed? It is tracked in both the start and stop
paths, but it does not appear to be evaluated or read anywhere else in the
driver, making it unused state tracking.

[ ... ]

> @@ -2033,10 +2033,10 @@ static enum hrtimer_restart rpi_axi_pmu_timer_handler(struct hrtimer *timer)
>  static void rpi_axi_pmu_start(struct perf_event *event, int flags)
>  {
>  	struct rpi_axi_pmu *pmu = pmu_to_rpi_axi_pmu(event->pmu);
>  	enum monitor mon = config_to_monitor(event->attr.config);
>  	enum counter counter = config_to_counter(event->attr.config);
>  	unsigned long spinflags;
>  
>  	raw_spin_lock_irqsave(&pmu->lock, spinflags);
>  	event->hw.state = 0;
>  	pmu->monitor[mon].hw_events.num_started++;

[ ... ]

> @@ -2123,13 +2123,13 @@ static int rpi_axi_pmu_add(struct perf_event *event, int flags)
>  static void rpi_axi_pmu_stop(struct perf_event *event, int flags)
>  {
>  	struct rpi_axi_pmu *pmu = pmu_to_rpi_axi_pmu(event->pmu);
>  	enum monitor mon = config_to_monitor(event->attr.config);
>  	unsigned long spinflags;
>  
>  	if (event->hw.state & PERF_HES_STOPPED)
>  		return;
>  
>  	if (flags & PERF_EF_UPDATE)
>  		rpi_axi_pmu_read(event);
>  
>  	raw_spin_lock_irqsave(&pmu->lock, spinflags);
>  	if (flags & PERF_EF_UPDATE)
>  		event->hw.state |= PERF_HES_UPTODATE;
>  	event->hw.state |= PERF_HES_STOPPED;
>  	pmu->monitor[mon].hw_events.num_started--;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1