| Message ID | 20200722172040.1299289-3-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Support for ETMv4.4 system instructions | expand |
On Wed, Jul 22, 2020 at 06:20:28PM +0100, Suzuki K Poulose wrote: > We are about to introduce support for sysreg access to ETMv4.4+ > component. Since there are generic routines that access the > registers (e.g, CS_LOCK/UNLOCK , claim/disclaim operations, timeout) > and in order to preserve the logic of these operations at a single place > we introduce an abstraction layer for the accesses to a given device. > This will also be helpful in consolidating the sysfs.attribute helpers, > that we define per driver. Please drop the last sentence, it doesn't add to the current patch. > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Mike Leach <mike.leach@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/hwtracing/coresight/coresight-catu.c | 1 + > drivers/hwtracing/coresight/coresight-cti.c | 1 + > drivers/hwtracing/coresight/coresight-etb10.c | 1 + > drivers/hwtracing/coresight/coresight-etm3x.c | 1 + > drivers/hwtracing/coresight/coresight-etm4x.c | 1 + > .../hwtracing/coresight/coresight-funnel.c | 1 + > .../coresight/coresight-replicator.c | 1 + > drivers/hwtracing/coresight/coresight-stm.c | 1 + > drivers/hwtracing/coresight/coresight-tmc.c | 1 + > drivers/hwtracing/coresight/coresight-tpiu.c | 1 + > drivers/hwtracing/coresight/coresight.c | 49 ++++++ > include/linux/coresight.h | 156 ++++++++++++++++++ > 12 files changed, 215 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c > index 1801804a7762..6299ff7b8a14 100644 > --- a/drivers/hwtracing/coresight/coresight-catu.c > +++ b/drivers/hwtracing/coresight/coresight-catu.c > @@ -551,6 +551,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) > dev->platform_data = pdata; > > drvdata->base = base; > + catu_desc.access.base = base; > catu_desc.pdata = pdata; > catu_desc.dev = dev; > catu_desc.groups = catu_groups; > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c > index 3ccc703dc940..c810ea3ba155 100644 > --- a/drivers/hwtracing/coresight/coresight-cti.c > +++ b/drivers/hwtracing/coresight/coresight-cti.c > @@ -865,6 +865,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR(base); > > drvdata->base = base; > + cti_desc.access.base = base; > > dev_set_drvdata(dev, drvdata); > > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c > index 03e3f2590191..0f2735e15119 100644 > --- a/drivers/hwtracing/coresight/coresight-etb10.c > +++ b/drivers/hwtracing/coresight/coresight-etb10.c > @@ -755,6 +755,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR(base); > > drvdata->base = base; > + desc.access.base = base; > > spin_lock_init(&drvdata->spinlock); > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c > index bf22dcfd3327..7ddcb7fcb2d6 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x.c > @@ -805,6 +805,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR(base); > > drvdata->base = base; > + desc.access.base = base; > > spin_lock_init(&drvdata->spinlock); > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c > index cb83fb77ded6..7bb74c659c4f 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > @@ -1501,6 +1501,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR(base); > > drvdata->base = base; > + desc.access.base = base; > > spin_lock_init(&drvdata->spinlock); > > diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c > index 900690a9f7f0..67fc3e3b77d8 100644 > --- a/drivers/hwtracing/coresight/coresight-funnel.c > +++ b/drivers/hwtracing/coresight/coresight-funnel.c > @@ -242,6 +242,7 @@ static int funnel_probe(struct device *dev, struct resource *res) > } > drvdata->base = base; > desc.groups = coresight_funnel_groups; > + desc.access.base = base; > } > > dev_set_drvdata(dev, drvdata); > diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c > index 78acf29c49ca..65704ada20a5 100644 > --- a/drivers/hwtracing/coresight/coresight-replicator.c > +++ b/drivers/hwtracing/coresight/coresight-replicator.c > @@ -254,6 +254,7 @@ static int replicator_probe(struct device *dev, struct resource *res) > } > drvdata->base = base; > desc.groups = replicator_groups; > + desc.access.base = base; > } > > if (fwnode_property_present(dev_fwnode(dev), > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c > index 673d2f56ed1e..c8509cc78512 100644 > --- a/drivers/hwtracing/coresight/coresight-stm.c > +++ b/drivers/hwtracing/coresight/coresight-stm.c > @@ -881,6 +881,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) > if (IS_ERR(base)) > return PTR_ERR(base); > drvdata->base = base; > + desc.access.base = base; > > ret = stm_get_stimulus_area(dev, &ch_res); > if (ret) > diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c > index 7040d583bed9..b49795ad6861 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.c > +++ b/drivers/hwtracing/coresight/coresight-tmc.c > @@ -458,6 +458,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > } > > drvdata->base = base; > + desc.access.base = base; > > spin_lock_init(&drvdata->spinlock); > > diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c > index f8583e4032a6..7ef7649f48ad 100644 > --- a/drivers/hwtracing/coresight/coresight-tpiu.c > +++ b/drivers/hwtracing/coresight/coresight-tpiu.c > @@ -149,6 +149,7 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR(base); > > drvdata->base = base; > + desc.access.base = base; > > /* Disable tpiu to support older devices */ > tpiu_disable_hw(drvdata); > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index e9c90f2de34a..38e9c03ab754 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -1387,6 +1387,54 @@ static int __init coresight_init(void) > } > postcore_initcall(coresight_init); > > +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) > +{ > + return csdev_access_relaxed_read32(&csdev->access, offset); > +} > + > +u32 coresight_read32(struct coresight_device *csdev, u32 offset) > +{ > + return csdev_access_read32(&csdev->access, offset); > +} > + > +void coresight_relaxed_write32(struct coresight_device *csdev, > + u32 val, > + u32 offset) > +{ > + Extra line > + csdev_access_relaxed_write32(&csdev->access, val, offset); > +} > + > + > +void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset) > +{ > + csdev_access_write32(&csdev->access, val, offset); > +} > + > +u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset) > +{ > + return csdev_access_relaxed_read64(&csdev->access, offset); > +} > + > +u64 coresight_read64(struct coresight_device *csdev, u32 offset) > +{ > + return csdev_access_read64(&csdev->access, offset); > +} > + > +void coresight_relaxed_write64(struct coresight_device *csdev, > + u64 val, > + u32 offset) > +{ > + Extra line > + csdev_access_relaxed_write64(&csdev->access, val, offset); > +} > + > + > +void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) > +{ > + csdev_access_write64(&csdev->access, val, offset); > +} > + > /* > * coresight_release_platform_data: Release references to the devices connected > * to the output port of this device. > @@ -1451,6 +1499,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > csdev->type = desc->type; > csdev->subtype = desc->subtype; > csdev->ops = desc->ops; > + csdev->access = desc->access; > csdev->orphan = false; > > csdev->dev.type = &coresight_dev_type[desc->type]; > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 58fffdecdbfd..81ac708689f8 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -7,6 +7,7 @@ > #define _LINUX_CORESIGHT_H > > #include <linux/device.h> > +#include <linux/io.h> > #include <linux/perf_event.h> > #include <linux/sched.h> > > @@ -114,6 +115,32 @@ struct coresight_platform_data { > struct coresight_connection *conns; > }; > > +/** > + * struct csdev_access - Abstraction of a CoreSight device access. > + * > + * @no_iomem : True if the device doesn't have iomem access. > + * @base : When no_iomem == false, base address of the component > + * @read : Read from the given "offset" of the given instance. > + * @write : Write "val" to the given "offset". > + */ > +struct csdev_access { > + bool no_iomem; I find the no_iomen to be difficult to understand, especially when prefixed with '!'. Using "has_iomem" would be a lot more intuitive and would avoid extra mental gymnastics. > + union { > + void __iomem *base; > + struct { > + u64 (*read)(struct csdev_access *csa, > + u32 offset, > + bool relaxed, > + bool _64bit); > + void (*write)(struct csdev_access *csa, > + u64 val, > + u32 offset, > + bool relaxed, > + bool _64bit); > + }; > + }; > +}; > + > /** > * struct coresight_desc - description of a component required from drivers > * @type: as defined by @coresight_dev_type. > @@ -134,6 +161,7 @@ struct coresight_desc { > struct device *dev; > const struct attribute_group **groups; > const char *name; > + struct csdev_access access; No documentation > }; > > /** > @@ -186,6 +214,7 @@ struct coresight_sysfs_link { > * @def_sink: cached reference to default sink found for this device. > * @ect_dev: Associated cross trigger device. Not part of the trace data > * path or connections. > + * @csa: Device i/o access abstraction for this device. > * @nr_links: number of sysfs links created to other components from this > * device. These will appear in the "connections" group. > * @has_conns_grp: Have added a "connections" group for sysfs links. > @@ -205,6 +234,7 @@ struct coresight_device { > struct coresight_device *def_sink; > /* cross trigger handling */ > struct coresight_device *ect_dev; > + struct csdev_access access; Documentation and field don't match. > /* sysfs links between components */ > int nr_links; > bool has_conns_grp; > @@ -324,6 +354,79 @@ struct coresight_ops { > const struct coresight_ops_ect *ect_ops; > }; > > +static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, > + u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return readl_relaxed(csa->base + offset); > + return csa->read(csa, offset, true, false); > +} > + > +static inline u64 csdev_access_relaxed_read64(struct csdev_access *csa, > + u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return readq_relaxed(csa->base + offset); > + return csa->read(csa, offset, true, true); > +} > + > +static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return readl(csa->base + offset); > + return csa->read(csa, offset, false, false); > +} > + > +static inline u64 csdev_access_read64(struct csdev_access *csa, u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return readq(csa->base + offset); > + return csa->read(csa, offset, false, true); > +} > + All of the above don't have a new line after the if() condition while the ones below do. Please pick a heurisitic and stick with it throughout. > +static inline void csdev_access_relaxed_write32(struct csdev_access *csa, > + u32 val, > + u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return writel_relaxed(val, csa->base + offset); > + > + return csa->write(csa, val, offset, true, false); > +} > + > +static inline void csdev_access_relaxed_write64(struct csdev_access *csa, > + u64 val, > + u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return writeq_relaxed(val, csa->base + offset); > + > + return csa->write(csa, val, offset, true, true); > +} > + > +static inline void csdev_access_write32(struct csdev_access *csa, u32 val, u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return writel(val, csa->base + offset); > + > + return csa->write(csa, val, offset, false, false); > +} > + > +static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 offset) > +{ > + if (likely(!csa->no_iomem)) > + return writeq(val, csa->base + offset); > + > + return csa->write(csa, val, offset, false, true); > +} > + > + > +u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset); > +u64 coresight_read64(struct coresight_device *csdev, u32 offset); > +void coresight_relaxed_write64(struct coresight_device *csdev, > + u64 val, u32 offset); > +void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset); > + > #ifdef CONFIG_CORESIGHT > extern struct coresight_device * > coresight_register(struct coresight_desc *desc); > @@ -342,6 +445,14 @@ extern char *coresight_alloc_device_name(struct coresight_dev_list *devs, > struct device *dev); > > extern bool coresight_loses_context_with_cpu(struct device *dev); > + > +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset); > +u32 coresight_read32(struct coresight_device *csdev, u32 offset); > +void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset); > +void coresight_relaxed_write32(struct coresight_device *csdev, > + u32 val, > + u32 offset); > + Why are the 64 bit version outside of the #ifdef and the 32 bit within? > #else > static inline struct coresight_device * > coresight_register(struct coresight_desc *desc) { return NULL; } > @@ -368,6 +479,51 @@ static inline bool coresight_loses_context_with_cpu(struct device *dev) > { > return false; > } > + > +static inline u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) > +{ > + WARN_ON_ONCE(1); > + return 0; > +} > + > +static inline u32 coresight_read32(struct coresight_device *csdev, u32 offset) > +{ > + WARN_ON_ONCE(1); > + return 0; > +} > + > +static void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset) > +{ > +} > + > +static void coresight_relaxed_write32(struct coresight_device *csdev, > + u32 val, u32 offset); > +{ > +} > + > +static inline u64 coresight_relaxed_read64(struct coresight_device *csdev, > + u32 offset) > +{ > + WARN_ON_ONCE(1); > + return 0; > +} > + > +static inline u64 coresight_read64(struct coresight_device *csdev, u32 offset) > +{ > + WARN_ON_ONCE(1); Not sure about the motivation behind using WARN_ON_ONCE(), and only in the read functions. I would simply return 0 here. After all if CONFIG_CORESIGHT is not defined they won't make it very far. > + return 0; > +} > + > +static inline void coresight_relaxed_write64(struct coresight_device *csdev, > + u64 val, > + u32 offset) > +{ > +} > + > +static inline void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) > +{ > +} > + > #endif I will likely come back to this patch once I have reviewed the rest of the set. > > extern int coresight_get_cpu(struct device *dev); > -- > 2.24.1 >
On 07/29/2020 08:56 PM, Mathieu Poirier wrote: > On Wed, Jul 22, 2020 at 06:20:28PM +0100, Suzuki K Poulose wrote: >> We are about to introduce support for sysreg access to ETMv4.4+ >> component. Since there are generic routines that access the >> registers (e.g, CS_LOCK/UNLOCK , claim/disclaim operations, timeout) >> and in order to preserve the logic of these operations at a single place >> we introduce an abstraction layer for the accesses to a given device. >> This will also be helpful in consolidating the sysfs.attribute helpers, >> that we define per driver. > > Please drop the last sentence, it doesn't add to the current patch. > Sure. >> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c >> index e9c90f2de34a..38e9c03ab754 100644 >> --- a/drivers/hwtracing/coresight/coresight.c >> +++ b/drivers/hwtracing/coresight/coresight.c >> @@ -1387,6 +1387,54 @@ static int __init coresight_init(void) >> } ... >> * coresight_release_platform_data: Release references to the devices connected >> * to the output port of this device. >> @@ -1451,6 +1499,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) >> csdev->type = desc->type; >> csdev->subtype = desc->subtype; >> csdev->ops = desc->ops; >> + csdev->access = desc->access; >> csdev->orphan = false; >> >> csdev->dev.type = &coresight_dev_type[desc->type]; >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >> index 58fffdecdbfd..81ac708689f8 100644 >> --- a/include/linux/coresight.h >> +++ b/include/linux/coresight.h >> @@ -7,6 +7,7 @@ >> #define _LINUX_CORESIGHT_H >> >> #include <linux/device.h> >> +#include <linux/io.h> >> #include <linux/perf_event.h> >> #include <linux/sched.h> >> >> @@ -114,6 +115,32 @@ struct coresight_platform_data { >> struct coresight_connection *conns; >> }; >> >> +/** >> + * struct csdev_access - Abstraction of a CoreSight device access. >> + * >> + * @no_iomem : True if the device doesn't have iomem access. >> + * @base : When no_iomem == false, base address of the component >> + * @read : Read from the given "offset" of the given instance. >> + * @write : Write "val" to the given "offset". >> + */ >> +struct csdev_access { >> + bool no_iomem; > > I find the no_iomen to be difficult to understand, especially when prefixed with > '!'. Using "has_iomem" would be a lot more intuitive and would avoid extra > mental gymnastics. I agree. That was a bit of laziness in part, to limit the changes to the existing drivers, where almost everyone, except the ETM would need to simply use the MMIO approach. So, in order to keep those changes to minimum, i.e, simply initialize the base, I used the inverted logic. I will fix it. >> +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset); >> +u32 coresight_read32(struct coresight_device *csdev, u32 offset); >> +void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset); >> +void coresight_relaxed_write32(struct coresight_device *csdev, >> + u32 val, >> + u32 offset); >> + > > Why are the 64 bit version outside of the #ifdef and the 32 bit within? Mistake ;-). I will address all of the comments above in my next version. ... >> +static inline u64 coresight_read64(struct coresight_device *csdev, u32 offset) >> +{ >> + WARN_ON_ONCE(1); > > Not sure about the motivation behind using WARN_ON_ONCE(), and only in the read > functions. I would simply return 0 here. After all if CONFIG_CORESIGHT is not > defined they won't make it very far. > If someone is reading the values, they might do something with the value, i.e, make a decision when they shouldn't. This is just to prevent such cases. >> + return 0; >> +} >> + >> +static inline void coresight_relaxed_write64(struct coresight_device *csdev, >> + u64 val, >> + u32 offset) >> +{ >> +} >> + >> +static inline void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) >> +{ >> +} >> + >> #endif > > I will likely come back to this patch once I have reviewed the rest of the set. > Sure. I am looking for thoughts on the proposed API (not ABI) changes, as they are quite significant and invasive changes in the code, without much functionality changes. Thank you for the review ! Suzuki
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 1801804a7762..6299ff7b8a14 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -551,6 +551,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) dev->platform_data = pdata; drvdata->base = base; + catu_desc.access.base = base; catu_desc.pdata = pdata; catu_desc.dev = dev; catu_desc.groups = catu_groups; diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 3ccc703dc940..c810ea3ba155 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -865,6 +865,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base; + cti_desc.access.base = base; dev_set_drvdata(dev, drvdata); diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 03e3f2590191..0f2735e15119 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -755,6 +755,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base; + desc.access.base = base; spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c index bf22dcfd3327..7ddcb7fcb2d6 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x.c @@ -805,6 +805,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base; + desc.access.base = base; spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index cb83fb77ded6..7bb74c659c4f 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -1501,6 +1501,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base; + desc.access.base = base; spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 900690a9f7f0..67fc3e3b77d8 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -242,6 +242,7 @@ static int funnel_probe(struct device *dev, struct resource *res) } drvdata->base = base; desc.groups = coresight_funnel_groups; + desc.access.base = base; } dev_set_drvdata(dev, drvdata); diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 78acf29c49ca..65704ada20a5 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -254,6 +254,7 @@ static int replicator_probe(struct device *dev, struct resource *res) } drvdata->base = base; desc.groups = replicator_groups; + desc.access.base = base; } if (fwnode_property_present(dev_fwnode(dev), diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index 673d2f56ed1e..c8509cc78512 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -881,6 +881,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(base)) return PTR_ERR(base); drvdata->base = base; + desc.access.base = base; ret = stm_get_stimulus_area(dev, &ch_res); if (ret) diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 7040d583bed9..b49795ad6861 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -458,6 +458,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) } drvdata->base = base; + desc.access.base = base; spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index f8583e4032a6..7ef7649f48ad 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -149,6 +149,7 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base; + desc.access.base = base; /* Disable tpiu to support older devices */ tpiu_disable_hw(drvdata); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index e9c90f2de34a..38e9c03ab754 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -1387,6 +1387,54 @@ static int __init coresight_init(void) } postcore_initcall(coresight_init); +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) +{ + return csdev_access_relaxed_read32(&csdev->access, offset); +} + +u32 coresight_read32(struct coresight_device *csdev, u32 offset) +{ + return csdev_access_read32(&csdev->access, offset); +} + +void coresight_relaxed_write32(struct coresight_device *csdev, + u32 val, + u32 offset) +{ + + csdev_access_relaxed_write32(&csdev->access, val, offset); +} + + +void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset) +{ + csdev_access_write32(&csdev->access, val, offset); +} + +u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset) +{ + return csdev_access_relaxed_read64(&csdev->access, offset); +} + +u64 coresight_read64(struct coresight_device *csdev, u32 offset) +{ + return csdev_access_read64(&csdev->access, offset); +} + +void coresight_relaxed_write64(struct coresight_device *csdev, + u64 val, + u32 offset) +{ + + csdev_access_relaxed_write64(&csdev->access, val, offset); +} + + +void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) +{ + csdev_access_write64(&csdev->access, val, offset); +} + /* * coresight_release_platform_data: Release references to the devices connected * to the output port of this device. @@ -1451,6 +1499,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->type = desc->type; csdev->subtype = desc->subtype; csdev->ops = desc->ops; + csdev->access = desc->access; csdev->orphan = false; csdev->dev.type = &coresight_dev_type[desc->type]; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 58fffdecdbfd..81ac708689f8 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -7,6 +7,7 @@ #define _LINUX_CORESIGHT_H #include <linux/device.h> +#include <linux/io.h> #include <linux/perf_event.h> #include <linux/sched.h> @@ -114,6 +115,32 @@ struct coresight_platform_data { struct coresight_connection *conns; }; +/** + * struct csdev_access - Abstraction of a CoreSight device access. + * + * @no_iomem : True if the device doesn't have iomem access. + * @base : When no_iomem == false, base address of the component + * @read : Read from the given "offset" of the given instance. + * @write : Write "val" to the given "offset". + */ +struct csdev_access { + bool no_iomem; + union { + void __iomem *base; + struct { + u64 (*read)(struct csdev_access *csa, + u32 offset, + bool relaxed, + bool _64bit); + void (*write)(struct csdev_access *csa, + u64 val, + u32 offset, + bool relaxed, + bool _64bit); + }; + }; +}; + /** * struct coresight_desc - description of a component required from drivers * @type: as defined by @coresight_dev_type. @@ -134,6 +161,7 @@ struct coresight_desc { struct device *dev; const struct attribute_group **groups; const char *name; + struct csdev_access access; }; /** @@ -186,6 +214,7 @@ struct coresight_sysfs_link { * @def_sink: cached reference to default sink found for this device. * @ect_dev: Associated cross trigger device. Not part of the trace data * path or connections. + * @csa: Device i/o access abstraction for this device. * @nr_links: number of sysfs links created to other components from this * device. These will appear in the "connections" group. * @has_conns_grp: Have added a "connections" group for sysfs links. @@ -205,6 +234,7 @@ struct coresight_device { struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev; + struct csdev_access access; /* sysfs links between components */ int nr_links; bool has_conns_grp; @@ -324,6 +354,79 @@ struct coresight_ops { const struct coresight_ops_ect *ect_ops; }; +static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, + u32 offset) +{ + if (likely(!csa->no_iomem)) + return readl_relaxed(csa->base + offset); + return csa->read(csa, offset, true, false); +} + +static inline u64 csdev_access_relaxed_read64(struct csdev_access *csa, + u32 offset) +{ + if (likely(!csa->no_iomem)) + return readq_relaxed(csa->base + offset); + return csa->read(csa, offset, true, true); +} + +static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset) +{ + if (likely(!csa->no_iomem)) + return readl(csa->base + offset); + return csa->read(csa, offset, false, false); +} + +static inline u64 csdev_access_read64(struct csdev_access *csa, u32 offset) +{ + if (likely(!csa->no_iomem)) + return readq(csa->base + offset); + return csa->read(csa, offset, false, true); +} + +static inline void csdev_access_relaxed_write32(struct csdev_access *csa, + u32 val, + u32 offset) +{ + if (likely(!csa->no_iomem)) + return writel_relaxed(val, csa->base + offset); + + return csa->write(csa, val, offset, true, false); +} + +static inline void csdev_access_relaxed_write64(struct csdev_access *csa, + u64 val, + u32 offset) +{ + if (likely(!csa->no_iomem)) + return writeq_relaxed(val, csa->base + offset); + + return csa->write(csa, val, offset, true, true); +} + +static inline void csdev_access_write32(struct csdev_access *csa, u32 val, u32 offset) +{ + if (likely(!csa->no_iomem)) + return writel(val, csa->base + offset); + + return csa->write(csa, val, offset, false, false); +} + +static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 offset) +{ + if (likely(!csa->no_iomem)) + return writeq(val, csa->base + offset); + + return csa->write(csa, val, offset, false, true); +} + + +u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset); +u64 coresight_read64(struct coresight_device *csdev, u32 offset); +void coresight_relaxed_write64(struct coresight_device *csdev, + u64 val, u32 offset); +void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset); + #ifdef CONFIG_CORESIGHT extern struct coresight_device * coresight_register(struct coresight_desc *desc); @@ -342,6 +445,14 @@ extern char *coresight_alloc_device_name(struct coresight_dev_list *devs, struct device *dev); extern bool coresight_loses_context_with_cpu(struct device *dev); + +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset); +u32 coresight_read32(struct coresight_device *csdev, u32 offset); +void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset); +void coresight_relaxed_write32(struct coresight_device *csdev, + u32 val, + u32 offset); + #else static inline struct coresight_device * coresight_register(struct coresight_desc *desc) { return NULL; } @@ -368,6 +479,51 @@ static inline bool coresight_loses_context_with_cpu(struct device *dev) { return false; } + +static inline u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) +{ + WARN_ON_ONCE(1); + return 0; +} + +static inline u32 coresight_read32(struct coresight_device *csdev, u32 offset) +{ + WARN_ON_ONCE(1); + return 0; +} + +static void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset) +{ +} + +static void coresight_relaxed_write32(struct coresight_device *csdev, + u32 val, u32 offset); +{ +} + +static inline u64 coresight_relaxed_read64(struct coresight_device *csdev, + u32 offset) +{ + WARN_ON_ONCE(1); + return 0; +} + +static inline u64 coresight_read64(struct coresight_device *csdev, u32 offset) +{ + WARN_ON_ONCE(1); + return 0; +} + +static inline void coresight_relaxed_write64(struct coresight_device *csdev, + u64 val, + u32 offset) +{ +} + +static inline void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) +{ +} + #endif extern int coresight_get_cpu(struct device *dev);
We are about to introduce support for sysreg access to ETMv4.4+ component. Since there are generic routines that access the registers (e.g, CS_LOCK/UNLOCK , claim/disclaim operations, timeout) and in order to preserve the logic of these operations at a single place we introduce an abstraction layer for the accesses to a given device. This will also be helpful in consolidating the sysfs.attribute helpers, that we define per driver. Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Mike Leach <mike.leach@linaro.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- drivers/hwtracing/coresight/coresight-catu.c | 1 + drivers/hwtracing/coresight/coresight-cti.c | 1 + drivers/hwtracing/coresight/coresight-etb10.c | 1 + drivers/hwtracing/coresight/coresight-etm3x.c | 1 + drivers/hwtracing/coresight/coresight-etm4x.c | 1 + .../hwtracing/coresight/coresight-funnel.c | 1 + .../coresight/coresight-replicator.c | 1 + drivers/hwtracing/coresight/coresight-stm.c | 1 + drivers/hwtracing/coresight/coresight-tmc.c | 1 + drivers/hwtracing/coresight/coresight-tpiu.c | 1 + drivers/hwtracing/coresight/coresight.c | 49 ++++++ include/linux/coresight.h | 156 ++++++++++++++++++ 12 files changed, 215 insertions(+)