Re: [PATCH 22/25] coresight: Use platform agnostic names

From: Mathieu Poirier
Date: Thu Mar 28 2019 - 15:43:33 EST

Next message: Doug Anderson: "Re: [PATCH AUTOSEL 5.0 011/262] tracing: kdb: Fix ftdump to not sleep"
Previous message: Stephen Hemminger: "Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Mar 20, 2019 at 06:49:39PM +0000, Suzuki K Poulose wrote:
> So far we have reused the name of the "platform" device for
> the CoreSight device. But this is not very intuitive when
> we move to ACPI. Also, the ACPI device names have ":" in them
> (e.g, ARMHC97C:01), which the perf tool doesn't like very much.
> This patch introduces a generic naming scheme, givin more intuitive
> names for the devices that appear on the CoreSight bus.
> The names follow the pattern "prefix" followed by "index" (e.g, etm5).
> We maintain a list of allocated devices per "prefix" to make sure
> we don't allocate a new name when it is reprobed (e.g, due to
> unsatisifed device dependencies). So, we maintain the list
> of "fwnodes" of the parent devices to allocate a consistent name.
> All devices except the ETMs get an index allocated in the order
> of probing. ETMs get an index based on the CPU they are attached to.
>
> TMC devices are named using "tmc_etf", "tmc_etb", and "tmc_etr"
> prefixes depending on the configuration of the device.
>
> The replicators are not classified as dynamic/static anymore.
> One could easily figure that out by checking the presence of
> "mgmt" registers under sysfs.
>
> Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> ---
> drivers/hwtracing/coresight/coresight-catu.c | 7 +++
> drivers/hwtracing/coresight/coresight-etb10.c | 6 +++
> drivers/hwtracing/coresight/coresight-etm3x.c | 5 ++
> drivers/hwtracing/coresight/coresight-etm4x.c | 4 ++
> drivers/hwtracing/coresight/coresight-funnel.c | 6 +++
> drivers/hwtracing/coresight/coresight-replicator.c | 7 +++
> drivers/hwtracing/coresight/coresight-stm.c | 6 +++
> drivers/hwtracing/coresight/coresight-tmc.c | 14 ++++++
> drivers/hwtracing/coresight/coresight-tpiu.c | 6 +++
> drivers/hwtracing/coresight/coresight.c | 58 ++++++++++++++++++++++
> include/linux/coresight.h | 25 +++++++++-
> 11 files changed, 143 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index 4595c67..6492bce 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -28,6 +28,8 @@
> #define catu_dbg(x, ...) do {} while (0)
> #endif
>
> +DEFINE_CORESIGHT_DEVLIST(catu_devs, "catu");
> +
> struct catu_etr_buf {
> struct tmc_sg_table *catu_table;
> dma_addr_t sladdr;
> @@ -510,6 +512,11 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id)
> ret = PTR_ERR(pdata);
> goto out;
> }
> +
> + pdata->name = coresight_alloc_device_name(&catu_devs, dev);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> dev->platform_data = pdata;
>
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index e4175849..42b525c 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -62,6 +62,8 @@
> #define ETB_FFSR_BIT 1
> #define ETB_FRAME_SIZE_WORDS 4
>
> +DEFINE_CORESIGHT_DEVLIST(etb10_devs, "etb10_");

I would drop the "10_" and just call it "etb". Below we have "tmc_etb" so there
is no chance of mistake.

> +
> /**
> * struct etb_drvdata - specifics associated to an ETB component
> * @base: memory mapped base address for this component.
> @@ -692,6 +694,10 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
> pdata = coresight_get_platform_data(dev);
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> + pdata->name = coresight_alloc_device_name(&etb10_devs, dev);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> adev->dev.platform_data = pdata;
>
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> index b101464..35ed953 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -797,6 +797,11 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
>
> + pdata->name = devm_kasprintf(dev, GFP_KERNEL,
> + "etm%d", pdata->cpu);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> adev->dev.platform_data = pdata;
> drvdata->use_cp14 = fwnode_property_read_bool(dev->fwnode, "arm,cp14");
> dev_set_drvdata(dev, drvdata);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index bfc23ab..55fcb78 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -982,6 +982,10 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
> pdata = coresight_get_platform_data(dev);
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> + pdata->name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", pdata->cpu);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> adev->dev.platform_data = pdata;
>
> dev_set_drvdata(dev, drvdata);
> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
> index 2590744..96b6773 100644
> --- a/drivers/hwtracing/coresight/coresight-funnel.c
> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
> @@ -27,6 +27,8 @@
> #define FUNNEL_HOLDTIME (0x7 << FUNNEL_HOLDTIME_SHFT)
> #define FUNNEL_ENSx_MASK 0xff
>
> +DEFINE_CORESIGHT_DEVLIST(funnel_devs, "funnel");
> +
> /**
> * struct funnel_drvdata - specifics associated to a funnel component
> * @base: memory mapped base address for this component.
> @@ -189,6 +191,10 @@ static int funnel_probe(struct amba_device *adev, const struct amba_id *id)
> pdata = coresight_get_platform_data(dev);
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> + pdata->name = coresight_alloc_device_name(&funnel_devs, dev);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> adev->dev.platform_data = pdata;
>
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 7eb3bf7..7b937c0 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -22,6 +22,8 @@
> #define REPLICATOR_IDFILTER0 0x000
> #define REPLICATOR_IDFILTER1 0x004
>
> +DEFINE_CORESIGHT_DEVLIST(replicator_devs, "replicator");
> +
> /**
> * struct replicator_drvdata - specifics associated to a replicator component
> * @base: memory mapped base address for this component. Also indicates
> @@ -184,6 +186,11 @@ static int replicator_probe(struct device *dev, struct resource *res)
> return PTR_ERR(pdata);
> dev->platform_data = pdata;
>
> + pdata->name = coresight_alloc_device_name(&replicator_devs, dev);
> +

Please remove the extra new line here.

> + if (!pdata->name)
> + return -ENOMEM;
> +
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> if (!drvdata)
> return -ENOMEM;
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 6514586..d94ae22 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -107,6 +107,8 @@ struct channel_space {
> unsigned long *guaranteed;
> };
>
> +DEFINE_CORESIGHT_DEVLIST(stm_devs, "stm");
> +
> /**
> * struct stm_drvdata - specifics associated to an STM component
> * @base: memory mapped base address for this component.
> @@ -813,6 +815,10 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
> pdata = coresight_get_platform_data(dev);
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> + pdata->name = coresight_alloc_device_name(&stm_devs, dev);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> adev->dev.platform_data = pdata;
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> if (!drvdata)
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 147ab17..030303d 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -25,6 +25,10 @@
> #include "coresight-priv.h"
> #include "coresight-tmc.h"
>
> +DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb");
> +DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf");
> +DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr");
> +
> void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
> {
> /* Ensure formatter, unformatter and hardware fifo are empty */
> @@ -394,6 +398,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> struct tmc_drvdata *drvdata;
> struct resource *res = &adev->res;
> struct coresight_desc desc = { 0 };
> + struct coresight_dev_list *dev_list = NULL;
>
> pdata = coresight_get_platform_data(dev);
> if (IS_ERR(pdata)) {
> @@ -440,6 +445,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> desc.type = CORESIGHT_DEV_TYPE_SINK;
> desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
> desc.ops = &tmc_etb_cs_ops;
> + dev_list = &etb_devs;
> break;
> case TMC_CONFIG_TYPE_ETR:
> desc.type = CORESIGHT_DEV_TYPE_SINK;
> @@ -449,11 +455,13 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> coresight_get_uci_data(id));
> if (ret)
> goto out;
> + dev_list = &etr_devs;
> break;
> case TMC_CONFIG_TYPE_ETF:
> desc.type = CORESIGHT_DEV_TYPE_LINKSINK;
> desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
> desc.ops = &tmc_etf_cs_ops;
> + dev_list = &etf_devs;
> break;
> default:
> pr_err("%s: Unsupported TMC config\n", pdata->name);
> @@ -461,6 +469,12 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> goto out;
> }
>
> + pdata->name = coresight_alloc_device_name(dev_list, dev);
> + if (!pdata->name) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
> index 18a749a..f5cc457 100644
> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> @@ -46,6 +46,8 @@
> #define FFCR_FON_MAN BIT(6)
> #define FFCR_STOP_FI BIT(12)
>
> +DEFINE_CORESIGHT_DEVLIST(tpiu_devs, "tpiu");
> +
> /**
> * @base: memory mapped base address for this component.
> * @dev: the coresight device entity associated to this component.
> @@ -124,6 +126,10 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id)
> pdata = coresight_get_platform_data(dev);
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> + pdata->name = coresight_alloc_device_name(&tpiu_devs, dev);
> + if (!pdata->name)
> + return -ENOMEM;
> +
> adev->dev.platform_data = pdata;
>
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index 9cdedab..ca40c55 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -1253,3 +1253,61 @@ void coresight_unregister(struct coresight_device *csdev)
> device_unregister(&csdev->dev);
> }
> EXPORT_SYMBOL_GPL(coresight_unregister);
> +
> +

Extra line

> +/*
> + * coresight_search_device_idx - Search the fwnode handle of a device
> + * in the given dev_idx list. Must be called with the coresight_mutex held.
> + *
> + * Returns the index of the entry, when found. Otherwise, -ENOENT.
> + */
> +static inline int coresight_search_device_idx(struct coresight_dev_list *dict,
> + struct fwnode_handle *fwnode)
> +{
> + int i;
> +
> + for (i = 0; i < dict->nr_idx; i++)
> + if (dict->fwnode_list[i] == fwnode)
> + return i;
> + return -ENOENT;
> +}
> +
> +/*
> + * coresight_alloc_device_name - Get an index for a given device in the
> + * device index list specific to a driver. An index is allocated for a
> + * device and is tracked with the fwnode_handle to prevent allocating
> + * duplicate indices for the same device (e.g, if we defer probing of
> + * a device due to dependencies), in case the index is requested again.
> + */
> +char *coresight_alloc_device_name(struct coresight_dev_list *dict,
> + struct device *dev)
> +{
> + int idx;
> + char *name = NULL;
> + struct fwnode_handle **list;
> +
> + mutex_lock(&coresight_mutex);
> +
> + idx = coresight_search_device_idx(dict, dev_fwnode(dev));
> + if (idx < 0) {
> + /* Make space for the new entry */
> + idx = dict->nr_idx;
> + list = krealloc(dict->fwnode_list,
> + (idx + 1) * sizeof(*dict->fwnode_list),
> + GFP_KERNEL);
> + if (ZERO_OR_NULL_PTR(list)) {
> + idx = -ENOMEM;

This is not needed.

> + goto done;
> + }
> +
> + list[idx] = dev_fwnode(dev);
> + dict->fwnode_list = list;
> + dict->nr_idx = idx + 1;
> + }
> +
> + name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", dict->pfx, idx);
> +done:
> + mutex_unlock(&coresight_mutex);
> + return name;
> +}
> +EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 76c31b2..9d933bb 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -174,6 +174,28 @@ struct coresight_device {
> struct dev_ext_attribute *ea;
> };
>
> +/*
> + * coresight_dev_list - Mapping for devices to "name" index for device
> + * names.
> + *
> + * @nr_idx: Number of entries already allocated.
> + * @pfx: Prefix pattern for device name.
> + * @fwnode_list: Array of fwnode_handles associated with each allocated
> + * index, upto nr_idx entries.
> + */
> +struct coresight_dev_list {
> + int nr_idx;
> + const char *pfx;
> + struct fwnode_handle **fwnode_list;
> +};
> +
> +#define DEFINE_CORESIGHT_DEVLIST(var, dev_pfx) \
> +static struct coresight_dev_list (var) = { \
> + .pfx = dev_pfx, \
> + .nr_idx = 0, \
> + .fwnode_list = NULL, \
> +}
> +

I like how you did this.

> #define to_coresight_device(d) container_of(d, struct coresight_device, dev)
>
> #define source_ops(csdev) csdev->ops->source_ops
> @@ -266,7 +288,8 @@ extern int coresight_claim_device_unlocked(void __iomem *base);
>
> extern void coresight_disclaim_device(void __iomem *base);
> extern void coresight_disclaim_device_unlocked(void __iomem *base);
> -
> +extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
> + struct device *dev);
> #else
> static inline struct coresight_device *
> coresight_register(struct coresight_desc *desc) { return NULL; }
> --
> 2.7.4
>

Next message: Doug Anderson: "Re: [PATCH AUTOSEL 5.0 011/262] tracing: kdb: Fix ftdump to not sleep"
Previous message: Stephen Hemminger: "Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]