| Message ID | 20230327050537.30861-4-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | coresight: etm4x: Migrate ACPI AMBA devices to platform driver | expand |
On 27/03/2023 06:05, Anshuman Khandual wrote: > Coresight device pid can be retrieved from its iomem base address, which is > stored in 'struct etm4x_drvdata'. This drops pid argument from etm4_probe() > and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives the > coresight device pid with a new helper coresight_get_pid(), right before it > is consumed in etm4_hisi_match_pid(). > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: Leo Yan <leo.yan@linaro.org> > Cc: coresight@lists.linaro.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > .../coresight/coresight-etm4x-core.c | 21 +++++++------------ > include/linux/coresight.h | 12 +++++++++++ > 2 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 5d77571a8df9..3521838ab4fb 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config *config); > static enum cpuhp_state hp_online; > > struct etm4_init_arg { > - unsigned int pid; > struct device *dev; > struct csdev_access *csa; > }; > @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) > } > > static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, > - unsigned int id) > + struct csdev_access *csa) > { > + unsigned int id = coresight_get_pid(csa); > + This throws up the following error on an ETE. ete: trying to read unsupported register @fe0 So, I guess this must be performed only for iomem based devices. System instruction based device must be identified by MIDR_EL1/REVIDR_EL1 if needed for specific erratum. This is not required now. So, we could bail out early if we are system instruction based device. > if (etm4_hisi_match_pid(id)) > set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); > } > @@ -385,7 +386,7 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) > } > > static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, > - unsigned int id) > + struct csdev_access *csa) > { > } > #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ > @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) > etm4_os_unlock_csa(drvdata, csa); > etm4_cs_unlock(drvdata, csa); > > - etm4_check_arch_features(drvdata, init_arg->pid); > + etm4_check_arch_features(drvdata, csa); > > /* find all capabilities of the tracing unit */ > etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); > @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) > return 0; > } > > -static int etm4_probe(struct device *dev, u32 etm_pid) > +static int etm4_probe(struct device *dev) > { > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); > struct csdev_access access = { 0 }; > @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 etm_pid) > > init_arg.dev = dev; > init_arg.csa = &access; > - init_arg.pid = etm_pid; > > /* > * Serialize against CPUHP callbacks to avoid race condition > @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) > > drvdata->base = base; > dev_set_drvdata(dev, drvdata); > - ret = etm4_probe(dev, id->id); > + ret = etm4_probe(dev); > if (!ret) > pm_runtime_put(&adev->dev); > > @@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > > - /* > - * System register based devices could match the > - * HW by reading appropriate registers on the HW > - * and thus we could skip the PID. > - */ > - ret = etm4_probe(&pdev->dev, 0); > + ret = etm4_probe(&pdev->dev); > > pm_runtime_put(&pdev->dev); > return ret; > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index f19a47b9bb5a..f85b041ea475 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -370,6 +370,18 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, > return csa->read(offset, true, false); > } > > +#define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) > + > +static inline u32 coresight_get_pid(struct csdev_access *csa) > +{ > + u32 i, pid = 0; > + > + for (i = 0; i < 4; i++) > + pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) << (i * 8); Given the above, we could make this iomem specific. Suzuki > + > + return pid; > +} > + > static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa, > u32 lo_offset, u32 hi_offset) > {
On 3/31/2023 4:06 AM, Suzuki K Poulose wrote: > On 27/03/2023 06:05, Anshuman Khandual wrote: >> Coresight device pid can be retrieved from its iomem base address, >> which is >> stored in 'struct etm4x_drvdata'. This drops pid argument from >> etm4_probe() >> and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives >> the >> coresight device pid with a new helper coresight_get_pid(), right >> before it >> is consumed in etm4_hisi_match_pid(). >> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >> Cc: Mike Leach <mike.leach@linaro.org> >> Cc: Leo Yan <leo.yan@linaro.org> >> Cc: coresight@lists.linaro.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> .../coresight/coresight-etm4x-core.c | 21 +++++++------------ >> include/linux/coresight.h | 12 +++++++++++ >> 2 files changed, 20 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> index 5d77571a8df9..3521838ab4fb 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config >> *config); >> static enum cpuhp_state hp_online; >> struct etm4_init_arg { >> - unsigned int pid; >> struct device *dev; >> struct csdev_access *csa; >> }; >> @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct >> etmv4_drvdata *drvdata) >> } >> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >> - unsigned int id) >> + struct csdev_access *csa) >> { >> + unsigned int id = coresight_get_pid(csa); >> + > > This throws up the following error on an ETE. > > ete: trying to read unsupported register @fe0 > > So, I guess this must be performed only for iomem based > devices. System instruction based device must be identified > by MIDR_EL1/REVIDR_EL1 if needed for specific erratum. > This is not required now. So, we could bail out early > if we are system instruction based device. Besides this, the PID is limited to (I think) 4 bits of ID. TRCIDRs offer revision information, but nothing manufacturer specific save for the designer. Register fields like MIDR_EL1 Variant + PartNum + Revision and TRCPIDR3 REVAND offer help. It may be a combination of registers are needed for a manufacturer to adequately ID a part to apply an erratum. Perhaps you could at least cache MIDR_EL1 for possible future use? > > >> if (etm4_hisi_match_pid(id)) >> set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); >> } >> @@ -385,7 +386,7 @@ static void etm4_disable_arch_specific(struct >> etmv4_drvdata *drvdata) >> } >> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >> - unsigned int id) >> + struct csdev_access *csa) >> { >> } >> #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ >> @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) >> etm4_os_unlock_csa(drvdata, csa); >> etm4_cs_unlock(drvdata, csa); >> - etm4_check_arch_features(drvdata, init_arg->pid); >> + etm4_check_arch_features(drvdata, csa); >> /* find all capabilities of the tracing unit */ >> etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); >> @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct >> etm4_init_arg *init_arg) >> return 0; >> } >> -static int etm4_probe(struct device *dev, u32 etm_pid) >> +static int etm4_probe(struct device *dev) >> { >> struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); >> struct csdev_access access = { 0 }; >> @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 >> etm_pid) >> init_arg.dev = dev; >> init_arg.csa = &access; >> - init_arg.pid = etm_pid; >> /* >> * Serialize against CPUHP callbacks to avoid race condition >> @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device >> *adev, const struct amba_id *id) >> drvdata->base = base; >> dev_set_drvdata(dev, drvdata); >> - ret = etm4_probe(dev, id->id); >> + ret = etm4_probe(dev); >> if (!ret) >> pm_runtime_put(&adev->dev); >> @@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct >> platform_device *pdev) >> pm_runtime_set_active(&pdev->dev); >> pm_runtime_enable(&pdev->dev); >> - /* >> - * System register based devices could match the >> - * HW by reading appropriate registers on the HW >> - * and thus we could skip the PID. >> - */ >> - ret = etm4_probe(&pdev->dev, 0); >> + ret = etm4_probe(&pdev->dev); >> pm_runtime_put(&pdev->dev); >> return ret; >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >> index f19a47b9bb5a..f85b041ea475 100644 >> --- a/include/linux/coresight.h >> +++ b/include/linux/coresight.h >> @@ -370,6 +370,18 @@ static inline u32 >> csdev_access_relaxed_read32(struct csdev_access *csa, >> return csa->read(offset, true, false); >> } >> +#define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) >> + >> +static inline u32 coresight_get_pid(struct csdev_access *csa) >> +{ >> + u32 i, pid = 0; >> + >> + for (i = 0; i < 4; i++) >> + pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) >> << (i * 8); > > Given the above, we could make this iomem specific. > > Suzuki > > >> + >> + return pid; >> +} >> + >> static inline u64 csdev_access_relaxed_read_pair(struct csdev_access >> *csa, >> u32 lo_offset, u32 hi_offset) >> { >
On 31/03/2023 22:24, Steve Clevenger wrote: > > > On 3/31/2023 4:06 AM, Suzuki K Poulose wrote: >> On 27/03/2023 06:05, Anshuman Khandual wrote: >>> Coresight device pid can be retrieved from its iomem base address, >>> which is >>> stored in 'struct etm4x_drvdata'. This drops pid argument from >>> etm4_probe() >>> and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives >>> the >>> coresight device pid with a new helper coresight_get_pid(), right >>> before it >>> is consumed in etm4_hisi_match_pid(). >>> >>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >>> Cc: Mike Leach <mike.leach@linaro.org> >>> Cc: Leo Yan <leo.yan@linaro.org> >>> Cc: coresight@lists.linaro.org >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >>> --- >>> .../coresight/coresight-etm4x-core.c | 21 +++++++------------ >>> include/linux/coresight.h | 12 +++++++++++ >>> 2 files changed, 20 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> index 5d77571a8df9..3521838ab4fb 100644 >>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config >>> *config); >>> static enum cpuhp_state hp_online; >>> struct etm4_init_arg { >>> - unsigned int pid; >>> struct device *dev; >>> struct csdev_access *csa; >>> }; >>> @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct >>> etmv4_drvdata *drvdata) >>> } >>> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >>> - unsigned int id) >>> + struct csdev_access *csa) >>> { >>> + unsigned int id = coresight_get_pid(csa); >>> + >> >> This throws up the following error on an ETE. >> >> ete: trying to read unsupported register @fe0 >> >> So, I guess this must be performed only for iomem based >> devices. System instruction based device must be identified >> by MIDR_EL1/REVIDR_EL1 if needed for specific erratum. >> This is not required now. So, we could bail out early >> if we are system instruction based device. > > Besides this, the PID is limited to (I think) 4 bits of ID. TRCIDRs > offer revision information, but nothing manufacturer specific save for > the designer. Register fields like MIDR_EL1 Variant + PartNum + Revision > and TRCPIDR3 REVAND offer help. It may be a combination of registers are > needed for a manufacturer to adequately ID a part to apply an erratum. > Perhaps you could at least cache MIDR_EL1 for possible future use? Like I said, if we ever need them, we could add it. I don't see a point in storing it right now, if we don't use it. Suzuki
On 3/31/23 16:36, Suzuki K Poulose wrote: > On 27/03/2023 06:05, Anshuman Khandual wrote: >> Coresight device pid can be retrieved from its iomem base address, which is >> stored in 'struct etm4x_drvdata'. This drops pid argument from etm4_probe() >> and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives the >> coresight device pid with a new helper coresight_get_pid(), right before it >> is consumed in etm4_hisi_match_pid(). >> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >> Cc: Mike Leach <mike.leach@linaro.org> >> Cc: Leo Yan <leo.yan@linaro.org> >> Cc: coresight@lists.linaro.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> .../coresight/coresight-etm4x-core.c | 21 +++++++------------ >> include/linux/coresight.h | 12 +++++++++++ >> 2 files changed, 20 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> index 5d77571a8df9..3521838ab4fb 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config *config); >> static enum cpuhp_state hp_online; >> struct etm4_init_arg { >> - unsigned int pid; >> struct device *dev; >> struct csdev_access *csa; >> }; >> @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) >> } >> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >> - unsigned int id) >> + struct csdev_access *csa) >> { >> + unsigned int id = coresight_get_pid(csa); >> + > > This throws up the following error on an ETE. > > ete: trying to read unsupported register @fe0 > > So, I guess this must be performed only for iomem based > devices. System instruction based device must be identified > by MIDR_EL1/REVIDR_EL1 if needed for specific erratum. > This is not required now. So, we could bail out early > if we are system instruction based device. Will bail out on non iomem devices via drvdata->base address switch. --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -373,9 +373,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { - unsigned int id = coresight_get_pid(csa); + if (!drvdata->base) + return; - if (etm4_hisi_match_pid(id)) + if (etm4_hisi_match_pid(coresight_get_pid(csa))) set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); } #else > > >> if (etm4_hisi_match_pid(id)) >> set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); >> } >> @@ -385,7 +386,7 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) >> } >> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >> - unsigned int id) >> + struct csdev_access *csa) >> { >> } >> #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ >> @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) >> etm4_os_unlock_csa(drvdata, csa); >> etm4_cs_unlock(drvdata, csa); >> - etm4_check_arch_features(drvdata, init_arg->pid); >> + etm4_check_arch_features(drvdata, csa); >> /* find all capabilities of the tracing unit */ >> etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); >> @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) >> return 0; >> } >> -static int etm4_probe(struct device *dev, u32 etm_pid) >> +static int etm4_probe(struct device *dev) >> { >> struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); >> struct csdev_access access = { 0 }; >> @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 etm_pid) >> init_arg.dev = dev; >> init_arg.csa = &access; >> - init_arg.pid = etm_pid; >> /* >> * Serialize against CPUHP callbacks to avoid race condition >> @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) >> drvdata->base = base; >> dev_set_drvdata(dev, drvdata); >> - ret = etm4_probe(dev, id->id); >> + ret = etm4_probe(dev); >> if (!ret) >> pm_runtime_put(&adev->dev); >> @@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) >> pm_runtime_set_active(&pdev->dev); >> pm_runtime_enable(&pdev->dev); >> - /* >> - * System register based devices could match the >> - * HW by reading appropriate registers on the HW >> - * and thus we could skip the PID. >> - */ >> - ret = etm4_probe(&pdev->dev, 0); >> + ret = etm4_probe(&pdev->dev); >> pm_runtime_put(&pdev->dev); >> return ret; >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >> index f19a47b9bb5a..f85b041ea475 100644 >> --- a/include/linux/coresight.h >> +++ b/include/linux/coresight.h >> @@ -370,6 +370,18 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, >> return csa->read(offset, true, false); >> } >> +#define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) >> + >> +static inline u32 coresight_get_pid(struct csdev_access *csa) >> +{ >> + u32 i, pid = 0; >> + >> + for (i = 0; i < 4; i++) >> + pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) << (i * 8); > > Given the above, we could make this iomem specific. We could change coresight_get_pid() to take iomem base address instead and fetch the pid. But is not the existing csdev_access based approach better and more generic ? #define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) static inline u32 coresight_get_pid(void __iomem *base) { u32 i, pid = 0; for (i = 0; i < 4; i++) pid |= readl(base + CORESIGHT_PIDRn(i)) << (i * 8); return pid; }
On 17/05/2023 05:32, Anshuman Khandual wrote: > > > On 3/31/23 16:36, Suzuki K Poulose wrote: >> On 27/03/2023 06:05, Anshuman Khandual wrote: >>> Coresight device pid can be retrieved from its iomem base address, which is >>> stored in 'struct etm4x_drvdata'. This drops pid argument from etm4_probe() >>> and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives the >>> coresight device pid with a new helper coresight_get_pid(), right before it >>> is consumed in etm4_hisi_match_pid(). >>> >>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >>> Cc: Mike Leach <mike.leach@linaro.org> >>> Cc: Leo Yan <leo.yan@linaro.org> >>> Cc: coresight@lists.linaro.org >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >>> --- >>> .../coresight/coresight-etm4x-core.c | 21 +++++++------------ >>> include/linux/coresight.h | 12 +++++++++++ >>> 2 files changed, 20 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> index 5d77571a8df9..3521838ab4fb 100644 >>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >>> @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config *config); >>> static enum cpuhp_state hp_online; >>> struct etm4_init_arg { >>> - unsigned int pid; >>> struct device *dev; >>> struct csdev_access *csa; >>> }; >>> @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) >>> } >>> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >>> - unsigned int id) >>> + struct csdev_access *csa) >>> { >>> + unsigned int id = coresight_get_pid(csa); >>> + >> >> This throws up the following error on an ETE. >> >> ete: trying to read unsupported register @fe0 >> >> So, I guess this must be performed only for iomem based >> devices. System instruction based device must be identified >> by MIDR_EL1/REVIDR_EL1 if needed for specific erratum. >> This is not required now. So, we could bail out early >> if we are system instruction based device. > > Will bail out on non iomem devices via drvdata->base address switch. > > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -373,9 +373,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) > static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, > struct csdev_access *csa) > { > - unsigned int id = coresight_get_pid(csa); > + if (!drvdata->base) > + return; I would use !csa->io_mem to be more readerf friendly. > > - if (etm4_hisi_match_pid(id)) > + if (etm4_hisi_match_pid(coresight_get_pid(csa))) > set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); > } > #else > >> >> >>> if (etm4_hisi_match_pid(id)) >>> set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); >>> } >>> @@ -385,7 +386,7 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) >>> } >>> static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, >>> - unsigned int id) >>> + struct csdev_access *csa) >>> { >>> } >>> #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ >>> @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) >>> etm4_os_unlock_csa(drvdata, csa); >>> etm4_cs_unlock(drvdata, csa); >>> - etm4_check_arch_features(drvdata, init_arg->pid); >>> + etm4_check_arch_features(drvdata, csa); >>> /* find all capabilities of the tracing unit */ >>> etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); >>> @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) >>> return 0; >>> } >>> -static int etm4_probe(struct device *dev, u32 etm_pid) >>> +static int etm4_probe(struct device *dev) >>> { >>> struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); >>> struct csdev_access access = { 0 }; >>> @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 etm_pid) >>> init_arg.dev = dev; >>> init_arg.csa = &access; >>> - init_arg.pid = etm_pid; >>> /* >>> * Serialize against CPUHP callbacks to avoid race condition >>> @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) >>> drvdata->base = base; >>> dev_set_drvdata(dev, drvdata); >>> - ret = etm4_probe(dev, id->id); >>> + ret = etm4_probe(dev); >>> if (!ret) >>> pm_runtime_put(&adev->dev); >>> @@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) >>> pm_runtime_set_active(&pdev->dev); >>> pm_runtime_enable(&pdev->dev); >>> - /* >>> - * System register based devices could match the >>> - * HW by reading appropriate registers on the HW >>> - * and thus we could skip the PID. >>> - */ >>> - ret = etm4_probe(&pdev->dev, 0); >>> + ret = etm4_probe(&pdev->dev); >>> pm_runtime_put(&pdev->dev); >>> return ret; >>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >>> index f19a47b9bb5a..f85b041ea475 100644 >>> --- a/include/linux/coresight.h >>> +++ b/include/linux/coresight.h >>> @@ -370,6 +370,18 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, >>> return csa->read(offset, true, false); >>> } >>> +#define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) >>> + >>> +static inline u32 coresight_get_pid(struct csdev_access *csa) >>> +{ >>> + u32 i, pid = 0; >>> + >>> + for (i = 0; i < 4; i++) >>> + pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) << (i * 8); >> >> Given the above, we could make this iomem specific. > > We could change coresight_get_pid() to take iomem base address instead > and fetch the pid. But is not the existing csdev_access based approach > better and more generic ? Yes, true, lets leave it with csdev_access, as it is coresight specific and not ETMv4. Cheers Suzuki
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 5d77571a8df9..3521838ab4fb 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -66,7 +66,6 @@ static u64 etm4_get_access_type(struct etmv4_config *config); static enum cpuhp_state hp_online; struct etm4_init_arg { - unsigned int pid; struct device *dev; struct csdev_access *csa; }; @@ -370,8 +369,10 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) } static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, - unsigned int id) + struct csdev_access *csa) { + unsigned int id = coresight_get_pid(csa); + if (etm4_hisi_match_pid(id)) set_bit(ETM4_IMPDEF_HISI_CORE_COMMIT, drvdata->arch_features); } @@ -385,7 +386,7 @@ static void etm4_disable_arch_specific(struct etmv4_drvdata *drvdata) } static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, - unsigned int id) + struct csdev_access *csa) { } #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ @@ -1165,7 +1166,7 @@ static void etm4_init_arch_data(void *info) etm4_os_unlock_csa(drvdata, csa); etm4_cs_unlock(drvdata, csa); - etm4_check_arch_features(drvdata, init_arg->pid); + etm4_check_arch_features(drvdata, csa); /* find all capabilities of the tracing unit */ etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); @@ -2048,7 +2049,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) return 0; } -static int etm4_probe(struct device *dev, u32 etm_pid) +static int etm4_probe(struct device *dev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct csdev_access access = { 0 }; @@ -2077,7 +2078,6 @@ static int etm4_probe(struct device *dev, u32 etm_pid) init_arg.dev = dev; init_arg.csa = &access; - init_arg.pid = etm_pid; /* * Serialize against CPUHP callbacks to avoid race condition @@ -2124,7 +2124,7 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) drvdata->base = base; dev_set_drvdata(dev, drvdata); - ret = etm4_probe(dev, id->id); + ret = etm4_probe(dev); if (!ret) pm_runtime_put(&adev->dev); @@ -2146,12 +2146,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); - /* - * System register based devices could match the - * HW by reading appropriate registers on the HW - * and thus we could skip the PID. - */ - ret = etm4_probe(&pdev->dev, 0); + ret = etm4_probe(&pdev->dev); pm_runtime_put(&pdev->dev); return ret; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f19a47b9bb5a..f85b041ea475 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -370,6 +370,18 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, return csa->read(offset, true, false); } +#define CORESIGHT_PIDRn(i) (0xFE0 + ((i) * 4)) + +static inline u32 coresight_get_pid(struct csdev_access *csa) +{ + u32 i, pid = 0; + + for (i = 0; i < 4; i++) + pid |= csdev_access_relaxed_read32(csa, CORESIGHT_PIDRn(i)) << (i * 8); + + return pid; +} + static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa, u32 lo_offset, u32 hi_offset) {
Coresight device pid can be retrieved from its iomem base address, which is stored in 'struct etm4x_drvdata'. This drops pid argument from etm4_probe() and 'struct etm4_init_arg'. Instead etm4_check_arch_features() derives the coresight device pid with a new helper coresight_get_pid(), right before it is consumed in etm4_hisi_match_pid(). Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: Mike Leach <mike.leach@linaro.org> Cc: Leo Yan <leo.yan@linaro.org> Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- .../coresight/coresight-etm4x-core.c | 21 +++++++------------ include/linux/coresight.h | 12 +++++++++++ 2 files changed, 20 insertions(+), 13 deletions(-)