On Fri, Apr 26, 2024 at 01:34:19PM -0700, Charlie Jenkins wrote:
On Fri, Apr 26, 2024 at 05:25:20PM +0100, Conor Dooley wrote:
On Sat, Apr 20, 2024 at 06:04:41PM -0700, Charlie Jenkins wrote:
Migrate xandespmu out of riscv_isa_ext and into a new Andes-specific vendor namespace. diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index 8cbe6e5f9c39..84760ce61e03 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -24,6 +24,8 @@ #include <asm/errata_list.h> #include <asm/sbi.h> #include <asm/cpufeature.h> +#include <asm/vendorid_list.h> +#include <asm/vendor_extensions/andes.h> #define ALT_SBI_PMU_OVERFLOW(__ovl) \ asm volatile(ALTERNATIVE_2( \ @@ -32,7 +34,7 @@ asm volatile(ALTERNATIVE_2( \ THEAD_VENDOR_ID, ERRATA_THEAD_PMU, \ CONFIG_ERRATA_THEAD_PMU, \ "csrr %0, " __stringify(ANDES_CSR_SCOUNTEROF), \
0, RISCV_ISA_EXT_XANDESPMU, \
CONFIG_ANDES_CUSTOM_PMU) \ : "=r" (__ovl) : \ : "memory")ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU, \
@@ -41,7 +43,7 @@ asm volatile(ALTERNATIVE_2( \ asm volatile(ALTERNATIVE( \ "csrc " __stringify(CSR_IP) ", %0\n\t", \ "csrc " __stringify(ANDES_CSR_SLIP) ", %0\n\t", \
0, RISCV_ISA_EXT_XANDESPMU, \
CONFIG_ANDES_CUSTOM_PMU) \ : : "r"(__irq_mask) \ : "memory")ANDES_VENDOR_ID, RISCV_ISA_VENDOR_EXT_XANDESPMU, \
@@ -837,7 +839,7 @@ static int pmu_sbi_setup_irqs(struct riscv_pmu *pmu, struct platform_device *pde riscv_cached_mimpid(0) == 0) { riscv_pmu_irq_num = THEAD_C9XX_RV_IRQ_PMU; riscv_pmu_use_irq = true;
- } else if (riscv_isa_extension_available(NULL, XANDESPMU) &&
- } else if (riscv_isa_vendor_extension_available(-1, XANDESPMU) &&
What's the rationale for this not using riscv_has_extension_unlikely()? Happens once in probe so don't bother? I forget if we discussed it when the code was added, but it would save us from the NULL/-1 syntax, neither of which I think is a good interface.
Doesn't look like something that was ever commented on in the series, but I may have missed it. I can change this to use the alternatives.
Yeha, not really a question for you but thinking aloud and wondering if someone would remind me. I really don't like riscv_isa_extension_available() because it doesn't respect config options etc, but ultimately I think the series that Clement is currently working on for Zc* is could be the saviour there, as the callbacks his most recent version has I think could make it much easier to hook in and turn off extensions. Should be helpful for the sort of confusing shit that Eric was complaining about last week on Andy's vector series.
This also wasn't supposed to be -1, it's supposed to be the id of the vendor.
Also, I'd prob drop the "drivers" from $subject.
I'll come back and look at the rest of this Monday, it's a sunny Friday here and I've still got my devicetree patch queue to clear..
- Charlie
Cheers, Conor.
IS_ENABLED(CONFIG_ANDES_CUSTOM_PMU)) { riscv_pmu_irq_num = ANDES_SLI_CAUSE_BASE + ANDES_RV_IRQ_PMOVI; riscv_pmu_use_irq = true;
-- 2.44.0