| Message ID | 1573267867-21991-1-git-send-email-zhangshaokun@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drivers/perf: hisi: Simplify hisi_read_sccl_and_ccl_id and its comment | expand |
On 09/11/2019 02:51, Shaokun Zhang wrote: > hisi_read_sccl_and_ccl_id is not readable That's a little strong :) and its comment is a little > confused, so simplify the function and its comment as Mark's suggestion. > > Cc: John Garry <john.garry@huawei.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Will Deacon <will@kernel.org> > Suggested-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com> > --- > drivers/perf/hisilicon/hisi_uncore_pmu.c | 58 ++++++++++++++++++-------------- > 1 file changed, 32 insertions(+), 26 deletions(-) > > diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c > index 96183e31b96a..9e9625a1f388 100644 > --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c > +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c > @@ -337,38 +337,44 @@ void hisi_uncore_pmu_disable(struct pmu *pmu) > hisi_pmu->ops->stop_counters(hisi_pmu); > } > > + > /* > - * Read Super CPU cluster and CPU cluster ID from MPIDR_EL1. > - * If multi-threading is supported, On Huawei Kunpeng 920 SoC whose cpu > - * core is tsv110, CCL_ID is the low 3-bits in MPIDR[Aff2] and SCCL_ID > - * is the upper 5-bits of Aff2 field; while for other cpu types, SCCL_ID > - * is in MPIDR[Aff3] and CCL_ID is in MPIDR[Aff2], if not, SCCL_ID > - * is in MPIDR[Aff2] and CCL_ID is in MPIDR[Aff1]. > + * The Super CPU Cluster (SCCL) and CPU Cluster (CCL) IDs can be > + * determined from the MPIDR_EL1, but the encoding varies by CPU: > + * > + * - For MT variants of TSV110 (e.g. found in Kunpeng 920): I wish that you would drop the "found in Kunpeng 920", as I find it confusing/misleading. Thanks, John > + * SCCL is Aff2[7:3], CCL is Aff2[2:0] > + * > + * - For other MT parts: > + * SCCL is Aff3[7:0], CCL is Aff2[7:0] > + * > + * - For non-MT parts: > + * SCCL is Aff2[7:0], CCL is Aff1[7:0] > */ > -static void hisi_read_sccl_and_ccl_id(int *sccl_id, int *ccl_id) > +static void hisi_read_sccl_and_ccl_id(int *scclp, int *cclp) > { > u64 mpidr = read_cpuid_mpidr(); > - > - if (mpidr & MPIDR_MT_BITMASK) { > - if (read_cpuid_part_number() == HISI_CPU_PART_TSV110) { > - int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2); > - > - if (sccl_id) > - *sccl_id = aff2 >> 3; > - if (ccl_id) > - *ccl_id = aff2 & 0x7; > - } else { > - if (sccl_id) > - *sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3); > - if (ccl_id) > - *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); > - } > + int aff3 = MPIDR_AFFINITY_LEVEL(mpidr, 3); > + int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2); > + int aff1 = MPIDR_AFFINITY_LEVEL(mpidr, 1); > + bool mt = mpidr & MPIDR_MT_BITMASK; > + int sccl, ccl; > + > + if (mt && read_cpuid_part_number() == HISI_CPU_PART_TSV110) { > + sccl = aff2 >> 3; > + ccl = aff2 & 0x7; > + } else if (mt) { > + sccl = aff3; > + ccl = aff2; > } else { > - if (sccl_id) > - *sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); > - if (ccl_id) > - *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 1); > + sccl = aff2; > + ccl = aff1; > } > + > + if (scclp) > + *scclp = sccl; > + if (cclp) > + *cclp = ccl; > } > > /* >
Hi John, On 2019/11/11 21:49, John Garry wrote: > On 09/11/2019 02:51, Shaokun Zhang wrote: >> hisi_read_sccl_and_ccl_id is not readable > > That's a little strong :) > > and its comment is a little >> confused, so simplify the function and its comment as Mark's suggestion. >> >> Cc: John Garry <john.garry@huawei.com> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Will Deacon <will@kernel.org> >> Suggested-by: Mark Rutland <mark.rutland@arm.com> >> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com> >> --- >> drivers/perf/hisilicon/hisi_uncore_pmu.c | 58 ++++++++++++++++++-------------- >> 1 file changed, 32 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c >> index 96183e31b96a..9e9625a1f388 100644 >> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c >> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c >> @@ -337,38 +337,44 @@ void hisi_uncore_pmu_disable(struct pmu *pmu) >> hisi_pmu->ops->stop_counters(hisi_pmu); >> } >> + >> /* >> - * Read Super CPU cluster and CPU cluster ID from MPIDR_EL1. >> - * If multi-threading is supported, On Huawei Kunpeng 920 SoC whose cpu >> - * core is tsv110, CCL_ID is the low 3-bits in MPIDR[Aff2] and SCCL_ID >> - * is the upper 5-bits of Aff2 field; while for other cpu types, SCCL_ID >> - * is in MPIDR[Aff3] and CCL_ID is in MPIDR[Aff2], if not, SCCL_ID >> - * is in MPIDR[Aff2] and CCL_ID is in MPIDR[Aff1]. >> + * The Super CPU Cluster (SCCL) and CPU Cluster (CCL) IDs can be >> + * determined from the MPIDR_EL1, but the encoding varies by CPU: >> + * >> + * - For MT variants of TSV110 (e.g. found in Kunpeng 920): > > I wish that you would drop the "found in Kunpeng 920", as I find it confusing/misleading. > How about * - For MT variants of TSV110 (e.g. found in Kunpeng 920 if the CPU variant is 0x1): ? If this is also confusing, I will drop it. Thanks, Shaokun > Thanks, > John > >> + * SCCL is Aff2[7:3], CCL is Aff2[2:0] >> + * >> + * - For other MT parts: >> + * SCCL is Aff3[7:0], CCL is Aff2[7:0] >> + * >> + * - For non-MT parts: >> + * SCCL is Aff2[7:0], CCL is Aff1[7:0] >> */ >> -static void hisi_read_sccl_and_ccl_id(int *sccl_id, int *ccl_id) >> +static void hisi_read_sccl_and_ccl_id(int *scclp, int *cclp) >> { >> u64 mpidr = read_cpuid_mpidr(); >> - >> - if (mpidr & MPIDR_MT_BITMASK) { >> - if (read_cpuid_part_number() == HISI_CPU_PART_TSV110) { >> - int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2); >> - >> - if (sccl_id) >> - *sccl_id = aff2 >> 3; >> - if (ccl_id) >> - *ccl_id = aff2 & 0x7; >> - } else { >> - if (sccl_id) >> - *sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3); >> - if (ccl_id) >> - *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); >> - } >> + int aff3 = MPIDR_AFFINITY_LEVEL(mpidr, 3); >> + int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2); >> + int aff1 = MPIDR_AFFINITY_LEVEL(mpidr, 1); >> + bool mt = mpidr & MPIDR_MT_BITMASK; >> + int sccl, ccl; >> + >> + if (mt && read_cpuid_part_number() == HISI_CPU_PART_TSV110) { >> + sccl = aff2 >> 3; >> + ccl = aff2 & 0x7; >> + } else if (mt) { >> + sccl = aff3; >> + ccl = aff2; >> } else { >> - if (sccl_id) >> - *sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); >> - if (ccl_id) >> - *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 1); >> + sccl = aff2; >> + ccl = aff1; >> } >> + >> + if (scclp) >> + *scclp = sccl; >> + if (cclp) >> + *cclp = ccl; >> } >> /* >> > > > . >
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c index 96183e31b96a..9e9625a1f388 100644 --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c @@ -337,38 +337,44 @@ void hisi_uncore_pmu_disable(struct pmu *pmu) hisi_pmu->ops->stop_counters(hisi_pmu); } + /* - * Read Super CPU cluster and CPU cluster ID from MPIDR_EL1. - * If multi-threading is supported, On Huawei Kunpeng 920 SoC whose cpu - * core is tsv110, CCL_ID is the low 3-bits in MPIDR[Aff2] and SCCL_ID - * is the upper 5-bits of Aff2 field; while for other cpu types, SCCL_ID - * is in MPIDR[Aff3] and CCL_ID is in MPIDR[Aff2], if not, SCCL_ID - * is in MPIDR[Aff2] and CCL_ID is in MPIDR[Aff1]. + * The Super CPU Cluster (SCCL) and CPU Cluster (CCL) IDs can be + * determined from the MPIDR_EL1, but the encoding varies by CPU: + * + * - For MT variants of TSV110 (e.g. found in Kunpeng 920): + * SCCL is Aff2[7:3], CCL is Aff2[2:0] + * + * - For other MT parts: + * SCCL is Aff3[7:0], CCL is Aff2[7:0] + * + * - For non-MT parts: + * SCCL is Aff2[7:0], CCL is Aff1[7:0] */ -static void hisi_read_sccl_and_ccl_id(int *sccl_id, int *ccl_id) +static void hisi_read_sccl_and_ccl_id(int *scclp, int *cclp) { u64 mpidr = read_cpuid_mpidr(); - - if (mpidr & MPIDR_MT_BITMASK) { - if (read_cpuid_part_number() == HISI_CPU_PART_TSV110) { - int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2); - - if (sccl_id) - *sccl_id = aff2 >> 3; - if (ccl_id) - *ccl_id = aff2 & 0x7; - } else { - if (sccl_id) - *sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3); - if (ccl_id) - *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); - } + int aff3 = MPIDR_AFFINITY_LEVEL(mpidr, 3); + int aff2 = MPIDR_AFFINITY_LEVEL(mpidr, 2); + int aff1 = MPIDR_AFFINITY_LEVEL(mpidr, 1); + bool mt = mpidr & MPIDR_MT_BITMASK; + int sccl, ccl; + + if (mt && read_cpuid_part_number() == HISI_CPU_PART_TSV110) { + sccl = aff2 >> 3; + ccl = aff2 & 0x7; + } else if (mt) { + sccl = aff3; + ccl = aff2; } else { - if (sccl_id) - *sccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); - if (ccl_id) - *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 1); + sccl = aff2; + ccl = aff1; } + + if (scclp) + *scclp = sccl; + if (cclp) + *cclp = ccl; } /*
hisi_read_sccl_and_ccl_id is not readable and its comment is a little confused, so simplify the function and its comment as Mark's suggestion. Cc: John Garry <john.garry@huawei.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Will Deacon <will@kernel.org> Suggested-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com> --- drivers/perf/hisilicon/hisi_uncore_pmu.c | 58 ++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 26 deletions(-)