CoreSight January 2022

coresight@lists.linaro.org

8 participants
47 discussions

Re: [PATCH v2 2/9] Coresight: Add coresight TPDM source driver

by Mathieu Poirier

Hi Jinlong, On Fri, Jan 21, 2022 at 10:01:47PM +0800, Jinlong Mao wrote: > Hi Mathieu, > > Good Day. > > On 12/17/2021 3:02 AM, Mathieu Poirier wrote: > > [...] > > > > > > > > + > > > > > > +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) > > > > > > +{ > > > > > > + static int traceid = TPDM_TRACE_ID_START; > > > > > > + > > > > > > + drvdata->traceid = traceid++; > > > > > > +} > > > > > I have been specific on how to properly do this in the last revision. Given the > > > > > above about the MAINTAINERS file, I am not sure that I will continue reviewing this set. > > > > > > > > > > There is also no need to rush another revision as I won't have the bandwidth to > > > > > process it before the holidays. > > > > > > > > > > Thanks, > > > > > Mathieu > > > > Hi Mathieu, > > > > > > > > Sorry, not addressed your previous comments here. > > > > > > > > For the trace id, each coresight component has 7 bits to store the trace > > > > id. So the trace id should be from 1 to 127 as 0 is invalid. > > > IDs 0x70 - 0x7F (`112 - 127 ) are reserved - see the ARM Coresight > > > Architecture specification v3.0 > > > > > Correct > > > > > > Apart from TPDMs/STM/ETMs, we also have other coresight components in > > > > our internal device. About 80 ids are already used. > > > > > > > > Some components have fixed trace id in HW. If we use functions below to > > > > count the trace id, there will be conflict to other components. > > > > > > > > Can we use 1-15 for etm trace ids and 16 - 127 for other coresight > > > > components ? And handle trace ids in its' own driver ? > > > > > > > This will limit systems to 15 cores - some have more! > > > > > Correct > > > > > > static inline int coresight_get_system_trace_id(int id) > > > > { > > > > /* Start system IDs above the highest per CPU trace ID. */ > > > > return coresigth_get_trace_id(cpumask_last(cpu_possible_mask) + 1); > > > > } > > Looking at my own suggestion again this won't work since it returns the same traceID > > when called multiple times. > > > > For this patchset and _without_ taking into account internal devices that have > > their traceID set in HW: > > > > 1. Define a bitmask that is 7 bit wide. > Should it be a 128 bit wide bitmask (0--127)? > > 2. By default, set bits under 0x10 and between 0x70 - 0x7F. > > 3. In coresight_get_system_trace_id(), drop the @id parameter and allocate the > > first available bit after cpumask_last(cpu_possible_mask) + 1. > > Should it allocate the first available bit after (cpumask_last(cpu_possible_mask) *2 ) + 0x10 ? > Return the first zero bit position as the trace id and set the bit. Please ignore my email from January 26th and proceed exactly as you stated above. I had conversations with Suzuki and Mike and providing an appropriate fix for this issue needs more investigation. Aplogies for the flip-flopping, as I said earlier this is a complex problem we've been restling with for quite some time now. Thanks, Mathieu > > > 4. Define a new function called coresight_put_system_trace_id(int id) that > > clears the bit in the mask corresponding to @id. > > > > For now that should work. > > > > > > static inline int coresight_get_trace_id(int cpu) > > > > { > > > > /* > > > > * A trace ID of value 0 is invalid, so let's start at some > > > > * random value that fits in 7 bits and go from there. Since > > > > * the common convention is to have data trace IDs be I(N) + 1, > > > > * set instruction trace IDs as a function of the CPU number. > > > > */ > > > > return (CORESIGHT_ETM_PMU_SEED + (cpu * 2)); > > > > } > > > > > > > This fixed relationship between cpu and trace ID is used in the perf > > > tooling to populate the elements in the perf.data file to correctly > > > allow association between CPU and trace data, and thus allow correct > > > trace decode. > > TraceIDs associated to CPUs are communicated to the perf tooling by way of the > > perf header - theoretically we should be able to change the allocation scheme > > without impacting the decoding process. > > > > > It should be possible to create another more dynamic mapping scheme - > > > but this must include a way to support the perf requirements too. > > > > > TraceIDs have been a lurking problem for as long as the subsystem has existed. > > For now what I have suggested above should be sufficient to provide an > > in-between solution that doesn't hold back this patchset. > > > > That being said, we need to start thinking about the best way to do this. I > > will put a patchset together in the new year that aims in that direction. > > > > > Regards > > > > > > Mike > > > > > > > Thanks > > > > > > > > Jinlong Mao > > > > > > > > > > + > > > > > > +static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > > > > > > +{ > > > > > > + struct device *dev = &adev->dev; > > > > > > + struct coresight_platform_data *pdata; > > > > > > + struct tpdm_drvdata *drvdata; > > > > > > + struct coresight_desc desc = { 0 }; > > > > > > + > > > > > > + desc.name = coresight_alloc_device_name(&tpdm_devs, dev); > > > > > > + if (!desc.name) > > > > > > + return -ENOMEM; > > > > > > + pdata = coresight_get_platform_data(dev); > > > > > > + if (IS_ERR(pdata)) > > > > > > + return PTR_ERR(pdata); > > > > > > + adev->dev.platform_data = pdata; > > > > > > + > > > > > > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > > > > > > + if (!drvdata) > > > > > > + return -ENOMEM; > > > > > > + drvdata->dev = &adev->dev; > > > > > > + dev_set_drvdata(dev, drvdata); > > > > > > + > > > > > > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > > > > > > + if (!drvdata->base) > > > > > > + return -ENOMEM; > > > > > > + > > > > > > + mutex_init(&drvdata->lock); > > > > > > + > > > > > > + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > > > > > > + desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SYS; > > > > > > + desc.ops = &tpdm_cs_ops; > > > > > > + desc.pdata = adev->dev.platform_data; > > > > > > + desc.dev = &adev->dev; > > > > > > + drvdata->csdev = coresight_register(&desc); > > > > > > + if (IS_ERR(drvdata->csdev)) > > > > > > + return PTR_ERR(drvdata->csdev); > > > > > > + > > > > > > + tpdm_init_default_data(drvdata); > > > > > > + pm_runtime_put(&adev->dev); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static void __exit tpdm_remove(struct amba_device *adev) > > > > > > +{ > > > > > > + struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > > > > > + > > > > > > + coresight_unregister(drvdata->csdev); > > > > > > +} > > > > > > + > > > > > > +static struct amba_id tpdm_ids[] = { > > > > > > + { > > > > > > + .id = 0x000f0e00, > > > > > > + .mask = 0x000fff00, > > > > > > + }, > > > > > > + { 0, 0}, > > > > > > +}; > > > > > > + > > > > > > +static struct amba_driver tpdm_driver = { > > > > > > + .drv = { > > > > > > + .name = "coresight-tpdm", > > > > > > + .owner = THIS_MODULE, > > > > > > + .suppress_bind_attrs = true, > > > > > > + }, > > > > > > + .probe = tpdm_probe, > > > > > > + .id_table = tpdm_ids, > > > > > > +}; > > > > > > + > > > > > > +module_amba_driver(tpdm_driver); > > > > > > + > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver"); > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > > > > > > new file mode 100644 > > > > > > index 000000000000..980ae90ff1c8 > > > > > > --- /dev/null > > > > > > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > > > > > > @@ -0,0 +1,31 @@ > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > +/* > > > > > > + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > > > > > > + */ > > > > > > + > > > > > > +#ifndef _CORESIGHT_CORESIGHT_TPDM_H > > > > > > +#define _CORESIGHT_CORESIGHT_TPDM_H > > > > > > + > > > > > > +/* Default value of the traceid */ > > > > > > +#define TPDM_TRACE_ID_START 128 > > > > > > + > > > > > > +/** > > > > > > + * struct tpdm_drvdata - specifics associated to an TPDM component > > > > > > + * @base: memory mapped base address for this component. > > > > > > + * @dev: The device entity associated to this component. > > > > > > + * @csdev: component vitals needed by the framework. > > > > > > + * @lock: lock for the enable value. > > > > > > + * @enable: enable status of the component. > > > > > > + * @traceid: value of the current ID for this component. > > > > > > + */ > > > > > > + > > > > > > +struct tpdm_drvdata { > > > > > > + void __iomem *base; > > > > > > + struct device *dev; > > > > > > + struct coresight_device *csdev; > > > > > > + struct mutex lock; > > > > > > + bool enable; > > > > > > + int traceid; > > > > > > +}; > > > > > > + > > > > > > +#endif /* _CORESIGHT_CORESIGHT_TPDM_H */ > > > > > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > > > > > > index 93a2922b7653..e48d463be63b 100644 > > > > > > --- a/include/linux/coresight.h > > > > > > +++ b/include/linux/coresight.h > > > > > > @@ -65,6 +65,7 @@ enum coresight_dev_subtype_source { > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_BUS, > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE, > > > > > > + CORESIGHT_DEV_SUBTYPE_SOURCE_SYS, > > > > > > }; > > > > > > > > > > > > enum coresight_dev_subtype_helper { > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > > -- > > > Mike Leach > > > Principal Engineer, ARM Ltd. > > > Manchester Design Centre. UK

2 years, 9 months

Re: [PATCH v2 2/9] Coresight: Add coresight TPDM source driver

by Mathieu Poirier

On Wed, Jan 26, 2022 at 03:07:38PM +0800, Jinlong Mao wrote: > Hi Mathieu, > > Good afternoon. > > On 1/22/2022 1:15 AM, Mathieu Poirier wrote: > > Hey Jinlong, > > > > On Fri, Jan 21, 2022 at 10:01:47PM +0800, Jinlong Mao wrote: > > > Hi Mathieu, > > > > > > Good Day. > > > > > > On 12/17/2021 3:02 AM, Mathieu Poirier wrote: > > > > [...] > > > > > > > > > > > > + > > > > > > > > +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) > > > > > > > > +{ > > > > > > > > + static int traceid = TPDM_TRACE_ID_START; > > > > > > > > + > > > > > > > > + drvdata->traceid = traceid++; > > > > > > > > +} > > > > > > > I have been specific on how to properly do this in the last revision. Given the > > > > > > > above about the MAINTAINERS file, I am not sure that I will continue reviewing this set. > > > > > > > > > > > > > > There is also no need to rush another revision as I won't have the bandwidth to > > > > > > > process it before the holidays. > > > > > > > > > > > > > > Thanks, > > > > > > > Mathieu > > > > > > Hi Mathieu, > > > > > > > > > > > > Sorry, not addressed your previous comments here. > > > > > > > > > > > > For the trace id, each coresight component has 7 bits to store the trace > > > > > > id. So the trace id should be from 1 to 127 as 0 is invalid. > > > > > IDs 0x70 - 0x7F (`112 - 127 ) are reserved - see the ARM Coresight > > > > > Architecture specification v3.0 > > > > > > > > > Correct > > > > > > > > > > Apart from TPDMs/STM/ETMs, we also have other coresight components in > > > > > > our internal device. About 80 ids are already used. > > > > > > > > > > > > Some components have fixed trace id in HW. If we use functions below to > > > > > > count the trace id, there will be conflict to other components. > > > > > > > > > > > > Can we use 1-15 for etm trace ids and 16 - 127 for other coresight > > > > > > components ? And handle trace ids in its' own driver ? > > > > > > > > > > > This will limit systems to 15 cores - some have more! > > > > > > > > > Correct > > > > > > > > > > static inline int coresight_get_system_trace_id(int id) > > > > > > { > > > > > > /* Start system IDs above the highest per CPU trace ID. */ > > > > > > return coresigth_get_trace_id(cpumask_last(cpu_possible_mask) + 1); > > > > > > } > > > > Looking at my own suggestion again this won't work since it returns the same traceID > > > > when called multiple times. > > > > > > > > For this patchset and _without_ taking into account internal devices that have > > > > their traceID set in HW: > > > > > > > > 1. Define a bitmask that is 7 bit wide. > > > Should it be a 128 bit wide bitmask (0--127)? > > Yes, you are correct. > > > > > > 2. By default, set bits under 0x10 and between 0x70 - 0x7F. > > > > 3. In coresight_get_system_trace_id(), drop the @id parameter and allocate the > > > > first available bit after cpumask_last(cpu_possible_mask) + 1. > > > Should it allocate the first available bit after (cpumask_last(cpu_possible_mask) *2 ) + 0x10 ? > > > Return the first zero bit position as the trace id and set the bit. > > I need to clarify something with Mike on this - I will get back to you on > > Monday. > Do you have more comments on this ? I just received clarifications on this - there is no need to continue enacting the current scheme and as such reserving bits following the above formula will not be needed either. Simply assigning free traceIDs based on request will work just fine. That of course is taking into account that 0x0, 0x1 and 0x70 - 0x7f are reserved. 0x1 is currently used for STM and should eventually be fixed to simply request a traceID the same way any other component do. You can fix it as part of this set but it is not mandatory. Let me know if there are things I haven't been clear on. Thanks, Mathieu > > > > > > 4. Define a new function called coresight_put_system_trace_id(int id) that > > > > clears the bit in the mask corresponding to @id. > > > > > > > > For now that should work. > > > > > > > > > > static inline int coresight_get_trace_id(int cpu) > > > > > > { > > > > > > /* > > > > > > * A trace ID of value 0 is invalid, so let's start at some > > > > > > * random value that fits in 7 bits and go from there. Since > > > > > > * the common convention is to have data trace IDs be I(N) + 1, > > > > > > * set instruction trace IDs as a function of the CPU number. > > > > > > */ > > > > > > return (CORESIGHT_ETM_PMU_SEED + (cpu * 2)); > > > > > > } > > > > > > > > > > > This fixed relationship between cpu and trace ID is used in the perf > > > > > tooling to populate the elements in the perf.data file to correctly > > > > > allow association between CPU and trace data, and thus allow correct > > > > > trace decode. > > > > TraceIDs associated to CPUs are communicated to the perf tooling by way of the > > > > perf header - theoretically we should be able to change the allocation scheme > > > > without impacting the decoding process. > > > > > > > > > It should be possible to create another more dynamic mapping scheme - > > > > > but this must include a way to support the perf requirements too. > > > > > > > > > TraceIDs have been a lurking problem for as long as the subsystem has existed. > > > > For now what I have suggested above should be sufficient to provide an > > > > in-between solution that doesn't hold back this patchset. > > > > > > > > That being said, we need to start thinking about the best way to do this. I > > > > will put a patchset together in the new year that aims in that direction. > > > > > > > > > Regards > > > > > > > > > > Mike > > > > > > > > > > > Thanks > > > > > > > > > > > > Jinlong Mao > > > > > > > > > > > > > > + > > > > > > > > +static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > > > > > > > > +{ > > > > > > > > + struct device *dev = &adev->dev; > > > > > > > > + struct coresight_platform_data *pdata; > > > > > > > > + struct tpdm_drvdata *drvdata; > > > > > > > > + struct coresight_desc desc = { 0 }; > > > > > > > > + > > > > > > > > + desc.name = coresight_alloc_device_name(&tpdm_devs, dev); > > > > > > > > + if (!desc.name) > > > > > > > > + return -ENOMEM; > > > > > > > > + pdata = coresight_get_platform_data(dev); > > > > > > > > + if (IS_ERR(pdata)) > > > > > > > > + return PTR_ERR(pdata); > > > > > > > > + adev->dev.platform_data = pdata; > > > > > > > > + > > > > > > > > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > > > > > > > > + if (!drvdata) > > > > > > > > + return -ENOMEM; > > > > > > > > + drvdata->dev = &adev->dev; > > > > > > > > + dev_set_drvdata(dev, drvdata); > > > > > > > > + > > > > > > > > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > > > > > > > > + if (!drvdata->base) > > > > > > > > + return -ENOMEM; > > > > > > > > + > > > > > > > > + mutex_init(&drvdata->lock); > > > > > > > > + > > > > > > > > + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > > > > > > > > + desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SYS; > > > > > > > > + desc.ops = &tpdm_cs_ops; > > > > > > > > + desc.pdata = adev->dev.platform_data; > > > > > > > > + desc.dev = &adev->dev; > > > > > > > > + drvdata->csdev = coresight_register(&desc); > > > > > > > > + if (IS_ERR(drvdata->csdev)) > > > > > > > > + return PTR_ERR(drvdata->csdev); > > > > > > > > + > > > > > > > > + tpdm_init_default_data(drvdata); > > > > > > > > + pm_runtime_put(&adev->dev); > > > > > > > > + > > > > > > > > + return 0; > > > > > > > > +} > > > > > > > > + > > > > > > > > +static void __exit tpdm_remove(struct amba_device *adev) > > > > > > > > +{ > > > > > > > > + struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > > > > > > > + > > > > > > > > + coresight_unregister(drvdata->csdev); > > > > > > > > +} > > > > > > > > + > > > > > > > > +static struct amba_id tpdm_ids[] = { > > > > > > > > + { > > > > > > > > + .id = 0x000f0e00, > > > > > > > > + .mask = 0x000fff00, > > > > > > > > + }, > > > > > > > > + { 0, 0}, > > > > > > > > +}; > > > > > > > > + > > > > > > > > +static struct amba_driver tpdm_driver = { > > > > > > > > + .drv = { > > > > > > > > + .name = "coresight-tpdm", > > > > > > > > + .owner = THIS_MODULE, > > > > > > > > + .suppress_bind_attrs = true, > > > > > > > > + }, > > > > > > > > + .probe = tpdm_probe, > > > > > > > > + .id_table = tpdm_ids, > > > > > > > > +}; > > > > > > > > + > > > > > > > > +module_amba_driver(tpdm_driver); > > > > > > > > + > > > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver"); > > > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..980ae90ff1c8 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > > > > > > > > @@ -0,0 +1,31 @@ > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > > > +/* > > > > > > > > + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > > > > > > > > + */ > > > > > > > > + > > > > > > > > +#ifndef _CORESIGHT_CORESIGHT_TPDM_H > > > > > > > > +#define _CORESIGHT_CORESIGHT_TPDM_H > > > > > > > > + > > > > > > > > +/* Default value of the traceid */ > > > > > > > > +#define TPDM_TRACE_ID_START 128 > > > > > > > > + > > > > > > > > +/** > > > > > > > > + * struct tpdm_drvdata - specifics associated to an TPDM component > > > > > > > > + * @base: memory mapped base address for this component. > > > > > > > > + * @dev: The device entity associated to this component. > > > > > > > > + * @csdev: component vitals needed by the framework. > > > > > > > > + * @lock: lock for the enable value. > > > > > > > > + * @enable: enable status of the component. > > > > > > > > + * @traceid: value of the current ID for this component. > > > > > > > > + */ > > > > > > > > + > > > > > > > > +struct tpdm_drvdata { > > > > > > > > + void __iomem *base; > > > > > > > > + struct device *dev; > > > > > > > > + struct coresight_device *csdev; > > > > > > > > + struct mutex lock; > > > > > > > > + bool enable; > > > > > > > > + int traceid; > > > > > > > > +}; > > > > > > > > + > > > > > > > > +#endif /* _CORESIGHT_CORESIGHT_TPDM_H */ > > > > > > > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > > > > > > > > index 93a2922b7653..e48d463be63b 100644 > > > > > > > > --- a/include/linux/coresight.h > > > > > > > > +++ b/include/linux/coresight.h > > > > > > > > @@ -65,6 +65,7 @@ enum coresight_dev_subtype_source { > > > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, > > > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_BUS, > > > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE, > > > > > > > > + CORESIGHT_DEV_SUBTYPE_SOURCE_SYS, > > > > > > > > }; > > > > > > > > > > > > > > > > enum coresight_dev_subtype_helper { > > > > > > > > -- > > > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > -- > > > > > Mike Leach > > > > > Principal Engineer, ARM Ltd. > > > > > Manchester Design Centre. UK

2 years, 9 months

Re: [PATCH] coresight: tmc: change device registration from misc to cdev

by Mathieu Poirier

On Tue, Jan 25, 2022 at 10:33:45AM +0800, Jiankang Chen wrote: > Hi Mathieu > > 在 2022/1/25 04:08, Mathieu Poirier 写道: > > On Mon, 24 Jan 2022 at 04:00, Jay Chen <jkchen(a)linux.alibaba.com> wrote: > > > Currently, there are 130 etr and etf on our machine, > > > but the current coresight tmc driver uses misc_register > > > to register the device, which leads to the error that > > > the device number is not enough. > > > > > > coresight-tmc: probe of xxxxx failed with error -16 > > > > > > This patch changes the device registration method > > > to cdev's dynamic registration method to solve the > > > problem of insufficient device numbers. > > This patch is still not labelled properly and as such being dropped. > > Hello, about the label of this patch, what kind of patch should I add? > So far you've sent 6 different version of this patch... Wouldn't it make sense to label this one "PATCH v7" ? Leo has pointed out a good presentation from Marc Zingier - I also suggest you invest time reading it. > Thanks very much > > jay > > > > Signed-off-by: Jay Chen <jkchen(a)linux.alibaba.com> > > > --- > > > .../hwtracing/coresight/coresight-tmc-core.c | 103 ++++++++++++++---- > > > drivers/hwtracing/coresight/coresight-tmc.h | 11 +- > > > 2 files changed, 91 insertions(+), 23 deletions(-) > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > > > index d0276af82494..0977f1ce6338 100644 > > > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > > > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > > > @@ -12,7 +12,6 @@ > > > #include <linux/io.h> > > > #include <linux/err.h> > > > #include <linux/fs.h> > > > -#include <linux/miscdevice.h> > > > #include <linux/mutex.h> > > > #include <linux/property.h> > > > #include <linux/uaccess.h> > > > @@ -31,6 +30,12 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > > > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > > > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > > > > > +static DEFINE_IDA(tmc_ida); > > > +static dev_t tmc_major; > > > +static struct class *tmc_class; > > > + > > > +#define TMC_DEV_MAX (MINORMASK + 1) > > > + > > > void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > > > { > > > struct coresight_device *csdev = drvdata->csdev; > > > @@ -146,8 +151,10 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata) > > > static int tmc_open(struct inode *inode, struct file *file) > > > { > > > int ret; > > > - struct tmc_drvdata *drvdata = container_of(file->private_data, > > > - struct tmc_drvdata, miscdev); > > > + struct tmc_cdev *cdev = container_of(inode->i_cdev, > > > + struct tmc_cdev, cdev); > > > + struct tmc_drvdata *drvdata = container_of(cdev, > > > + struct tmc_drvdata, cdev); > > > > > > ret = tmc_read_prepare(drvdata); > > > if (ret) > > > @@ -178,8 +185,10 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, > > > { > > > char *bufp; > > > ssize_t actual; > > > - struct tmc_drvdata *drvdata = container_of(file->private_data, > > > - struct tmc_drvdata, miscdev); > > > + struct tmc_cdev *cdev = container_of(file->f_inode->i_cdev, > > > + struct tmc_cdev, cdev); > > > + struct tmc_drvdata *drvdata = container_of(cdev, > > > + struct tmc_drvdata, cdev); > > > actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp); > > > if (actual <= 0) > > > return 0; > > > @@ -199,8 +208,10 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, > > > static int tmc_release(struct inode *inode, struct file *file) > > > { > > > int ret; > > > - struct tmc_drvdata *drvdata = container_of(file->private_data, > > > - struct tmc_drvdata, miscdev); > > > + struct tmc_cdev *cdev = container_of(inode->i_cdev, > > > + struct tmc_cdev, cdev); > > > + struct tmc_drvdata *drvdata = container_of(cdev, > > > + struct tmc_drvdata, cdev); > > > > > > ret = tmc_read_unprepare(drvdata); > > > if (ret) > > > @@ -451,6 +462,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > > > { > > > int ret = 0; > > > u32 devid; > > > + dev_t devt; > > > void __iomem *base; > > > struct device *dev = &adev->dev; > > > struct coresight_platform_data *pdata = NULL; > > > @@ -546,14 +558,32 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > > > goto out; > > > } > > > > > > - drvdata->miscdev.name = desc.name; > > > - drvdata->miscdev.minor = MISC_DYNAMIC_MINOR; > > > - drvdata->miscdev.fops = &tmc_fops; > > > - ret = misc_register(&drvdata->miscdev); > > > + ret = ida_simple_get(&tmc_ida, 0, TMC_DEV_MAX, GFP_KERNEL); > > > + if (ret < 0) > > > + goto err_coresight_unregister; > > > + > > > + cdev_init(&drvdata->cdev.cdev, &tmc_fops); > > > + drvdata->cdev.cdev.owner = THIS_MODULE; > > > + devt = MKDEV(MAJOR(tmc_major), ret); > > > + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); > > > if (ret) > > > - coresight_unregister(drvdata->csdev); > > > - else > > > + goto err_free_tmc_ida; > > > + > > > + drvdata->cdev.dev = device_create(tmc_class, NULL, devt, &drvdata->cdev, desc.name); > > > + if (IS_ERR(drvdata->cdev.dev)) { > > > + ret = PTR_ERR(drvdata->cdev.dev); > > > + goto err_delete_cdev; > > > + } else > > > pm_runtime_put(&adev->dev); > > > + > > > + return 0; > > > + > > > +err_delete_cdev: > > > + cdev_del(&drvdata->cdev.cdev); > > > +err_free_tmc_ida: > > > + ida_simple_remove(&tmc_ida, MINOR(devt)); > > > +err_coresight_unregister: > > > + coresight_unregister(drvdata->csdev); > > > out: > > > return ret; > > > } > > > @@ -583,13 +613,11 @@ static void tmc_shutdown(struct amba_device *adev) > > > static void tmc_remove(struct amba_device *adev) > > > { > > > struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > > + struct device *dev = drvdata->cdev.dev; > > > > > > - /* > > > - * Since misc_open() holds a refcount on the f_ops, which is > > > - * etb fops in this case, device is there until last file > > > - * handler to this device is closed. > > > - */ > > > - misc_deregister(&drvdata->miscdev); > > > + ida_simple_remove(&tmc_ida, MINOR(dev->devt)); > > > + device_destroy(tmc_class, dev->devt); > > > + cdev_del(&drvdata->cdev.cdev); > > > coresight_unregister(drvdata->csdev); > > > } > > > > > > @@ -618,7 +646,42 @@ static struct amba_driver tmc_driver = { > > > .id_table = tmc_ids, > > > }; > > > > > > -module_amba_driver(tmc_driver); > > > +static int __init tmc_init(void) > > > +{ > > > + int ret; > > > + > > > + ret = alloc_chrdev_region(&tmc_major, 0, TMC_DEV_MAX, "tmc"); > > > + if (ret < 0) { > > > + pr_err("tmc: failed to allocate char dev region\n"); > > > + return ret; > > > + } > > > + > > > + tmc_class = class_create(THIS_MODULE, "tmc"); > > > + if (IS_ERR(tmc_class)) { > > > + pr_err("tmc: failed to create class\n"); > > > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > > > + return PTR_ERR(tmc_class); > > > + } > > > + > > > + ret = amba_driver_register(&tmc_driver); > > > + if (ret) { > > > + pr_err("tmc: error registering amba driver\n"); > > > + class_destroy(tmc_class); > > > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +static void __exit tmc_exit(void) > > > +{ > > > + amba_driver_unregister(&tmc_driver); > > > + class_destroy(tmc_class); > > > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > > > +} > > > + > > > +module_init(tmc_init); > > > +module_exit(tmc_exit); > > > > > > MODULE_AUTHOR("Pratik Patel <pratikp(a)codeaurora.org>"); > > > MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver"); > > > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > > > index 6bec20a392b3..11441c6b9c26 100644 > > > --- a/drivers/hwtracing/coresight/coresight-tmc.h > > > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > > > @@ -7,9 +7,9 @@ > > > #ifndef _CORESIGHT_TMC_H > > > #define _CORESIGHT_TMC_H > > > > > > +#include <linux/cdev.h> > > > #include <linux/dma-mapping.h> > > > #include <linux/idr.h> > > > -#include <linux/miscdevice.h> > > > #include <linux/mutex.h> > > > #include <linux/refcount.h> > > > > > > @@ -163,11 +163,16 @@ struct etr_buf { > > > void *private; > > > }; > > > > > > +struct tmc_cdev { > > > + struct cdev cdev; > > > + struct device *dev; > > > +}; > > > + > > > /** > > > * struct tmc_drvdata - specifics associated to an TMC component > > > * @base: memory mapped base address for this component. > > > * @csdev: component vitals needed by the framework. > > > - * @miscdev: specifics to handle "/dev/xyz.tmc" entry. > > > + * @tmc_cdev: specifics to handle "/dev/xyz.tmc" entry. > > > * @spinlock: only one at a time pls. > > > * @pid: Process ID of the process being monitored by the session > > > * that is using this component. > > > @@ -191,7 +196,7 @@ struct etr_buf { > > > struct tmc_drvdata { > > > void __iomem *base; > > > struct coresight_device *csdev; > > > - struct miscdevice miscdev; > > > + struct tmc_cdev cdev; > > > spinlock_t spinlock; > > > pid_t pid; > > > bool reading; > > > -- > > > 2.27.0 > > >

2 years, 9 months

Re: [PATCH V3 0/7] coresight: trbe: Workaround Cortex-A510 erratas

by Suzuki K Poulose

On 25/01/2022 03:21, Anshuman Khandual wrote: > > > On 1/11/22 9:08 AM, Anshuman Khandual wrote: >> This series adds three different workarounds in the TRBE driver for >> Cortex-A510 specific erratas. But first, this adds Cortex-A510 specific cpu >> part number definition in the platform. This series applies on 5.16. >> >> Relevant errata documents can be found here. >> >> https://developer.arm.com/documentation/SDEN2397239/900 >> https://developer.arm.com/documentation/SDEN2397589/900 >> >> Changes in V3: >> >> - Moved the comment inside trbe_needs_drain_after_disable() >> - Moved the comment inside trbe_needs_ctxt_sync_after_enable() >> >> Changes in V2: >> >> https://lore.kernel.org/all/1641517808-5735-1-git-send-email-anshuman.khand… >> >> Accommodated most review comments from the previous version. >> >> - Split all patches into CPU errata definition, detection and TRBE workarounds >> - s/TRBE_WORKAROUND_SYSREG_WRITE_FAILURE/TRBE_NEEDS_DRAIN_AFTER_DISABLE >> - s/TRBE_WORKAROUND_CORRUPTION_WITH_ENABLE/TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE >> - s/trbe_may_fail_sysreg_write()/trbe_needs_drain_after_disable() >> - s/trbe_may_corrupt_with_enable()/trbe_needs_ctxt_sync_after_enable() >> - Updated Kconfig help message for config ARM64_ERRATUM_1902691 >> - Updated error message for trbe_is_broken() detection >> - Added new trblimitr parameter to set_trbe_enabled(), improving performance >> - Added COMPILE_TEST dependency in the errata, until TRBE part is available >> >> Changes in V1: >> >> https://lore.kernel.org/lkml/1641359159-22726-1-git-send-email-anshuman.kha… >> >> Cc: Catalin Marinas <catalin.marinas(a)arm.com> >> Cc: Will Deacon <will(a)kernel.org> >> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> >> Cc: Suzuki Poulose <suzuki.poulose(a)arm.com> >> Cc: coresight(a)lists.linaro.org >> Cc: linux-doc(a)vger.kernel.org >> Cc: linux-arm-kernel(a)lists.infradead.org >> Cc: linux-kernel(a)vger.kernel.org >> >> Anshuman Khandual (7): >> arm64: Add Cortex-A510 CPU part definition >> arm64: errata: Add detection for TRBE ignored system register writes >> arm64: errata: Add detection for TRBE invalid prohibited states >> arm64: errata: Add detection for TRBE trace data corruption >> coresight: trbe: Work around the ignored system register writes >> coresight: trbe: Work around the invalid prohibited states >> coresight: trbe: Work around the trace data corruption > > Hello Catalin/Mathieu, > > I am wondering how this series is going to be merged i.e via arm64 or coresight > tree ? Also will this require rebasing (and resend) against v5.17-rc1 release. > Please do suggest. Thank you. I would recommend rebasing on rc1. Thanks Suzuki

2 years, 9 months

Re: [PATCH] coresight: tmc: change device registration from misc to cdev

by Leo Yan

Hi Jay, On Tue, Jan 25, 2022 at 10:33:45AM +0800, Jiankang Chen wrote: > Hi Mathieu > > 在 2022/1/25 04:08, Mathieu Poirier 写道: > > On Mon, 24 Jan 2022 at 04:00, Jay Chen <jkchen(a)linux.alibaba.com> wrote: > > > Currently, there are 130 etr and etf on our machine, > > > but the current coresight tmc driver uses misc_register > > > to register the device, which leads to the error that > > > the device number is not enough. > > > > > > coresight-tmc: probe of xxxxx failed with error -16 > > > > > > This patch changes the device registration method > > > to cdev's dynamic registration method to solve the > > > problem of insufficient device numbers. > > This patch is still not labelled properly and as such being dropped. > > Hello, about the label of this patch, what kind of patch should I add? You could use "git format" command with option "-v" to add the version number, e.g. if you are committing patch version 4, so you can generate patch like: $ git format-patch HEAD~1 -v4 Or if you want to generate patch for a specific commit: $ git format-patch -1 patch_commit -v4 Every time when you generate a new version's patch, you could increment the version number with option "-v4", "-v5", and so on. P.s. I strongly suggest Marc Zyngier's great talk for how to upstream kernel patch in a good way: https://events19.linuxfoundation.org/wp-content/uploads/2017/12/Getting-You… Thanks, Leo

2 years, 9 months

Re: [PATCH] coresight: tmc: change device registration from misc to cdev

by Mathieu Poirier

On Mon, 24 Jan 2022 at 04:00, Jay Chen <jkchen(a)linux.alibaba.com> wrote: > > Currently, there are 130 etr and etf on our machine, > but the current coresight tmc driver uses misc_register > to register the device, which leads to the error that > the device number is not enough. > > coresight-tmc: probe of xxxxx failed with error -16 > > This patch changes the device registration method > to cdev's dynamic registration method to solve the > problem of insufficient device numbers. This patch is still not labelled properly and as such being dropped. > > Signed-off-by: Jay Chen <jkchen(a)linux.alibaba.com> > --- > .../hwtracing/coresight/coresight-tmc-core.c | 103 ++++++++++++++---- > drivers/hwtracing/coresight/coresight-tmc.h | 11 +- > 2 files changed, 91 insertions(+), 23 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index d0276af82494..0977f1ce6338 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -12,7 +12,6 @@ > #include <linux/io.h> > #include <linux/err.h> > #include <linux/fs.h> > -#include <linux/miscdevice.h> > #include <linux/mutex.h> > #include <linux/property.h> > #include <linux/uaccess.h> > @@ -31,6 +30,12 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > +static DEFINE_IDA(tmc_ida); > +static dev_t tmc_major; > +static struct class *tmc_class; > + > +#define TMC_DEV_MAX (MINORMASK + 1) > + > void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > { > struct coresight_device *csdev = drvdata->csdev; > @@ -146,8 +151,10 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata) > static int tmc_open(struct inode *inode, struct file *file) > { > int ret; > - struct tmc_drvdata *drvdata = container_of(file->private_data, > - struct tmc_drvdata, miscdev); > + struct tmc_cdev *cdev = container_of(inode->i_cdev, > + struct tmc_cdev, cdev); > + struct tmc_drvdata *drvdata = container_of(cdev, > + struct tmc_drvdata, cdev); > > ret = tmc_read_prepare(drvdata); > if (ret) > @@ -178,8 +185,10 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, > { > char *bufp; > ssize_t actual; > - struct tmc_drvdata *drvdata = container_of(file->private_data, > - struct tmc_drvdata, miscdev); > + struct tmc_cdev *cdev = container_of(file->f_inode->i_cdev, > + struct tmc_cdev, cdev); > + struct tmc_drvdata *drvdata = container_of(cdev, > + struct tmc_drvdata, cdev); > actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp); > if (actual <= 0) > return 0; > @@ -199,8 +208,10 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, > static int tmc_release(struct inode *inode, struct file *file) > { > int ret; > - struct tmc_drvdata *drvdata = container_of(file->private_data, > - struct tmc_drvdata, miscdev); > + struct tmc_cdev *cdev = container_of(inode->i_cdev, > + struct tmc_cdev, cdev); > + struct tmc_drvdata *drvdata = container_of(cdev, > + struct tmc_drvdata, cdev); > > ret = tmc_read_unprepare(drvdata); > if (ret) > @@ -451,6 +462,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > { > int ret = 0; > u32 devid; > + dev_t devt; > void __iomem *base; > struct device *dev = &adev->dev; > struct coresight_platform_data *pdata = NULL; > @@ -546,14 +558,32 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > goto out; > } > > - drvdata->miscdev.name = desc.name; > - drvdata->miscdev.minor = MISC_DYNAMIC_MINOR; > - drvdata->miscdev.fops = &tmc_fops; > - ret = misc_register(&drvdata->miscdev); > + ret = ida_simple_get(&tmc_ida, 0, TMC_DEV_MAX, GFP_KERNEL); > + if (ret < 0) > + goto err_coresight_unregister; > + > + cdev_init(&drvdata->cdev.cdev, &tmc_fops); > + drvdata->cdev.cdev.owner = THIS_MODULE; > + devt = MKDEV(MAJOR(tmc_major), ret); > + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); > if (ret) > - coresight_unregister(drvdata->csdev); > - else > + goto err_free_tmc_ida; > + > + drvdata->cdev.dev = device_create(tmc_class, NULL, devt, &drvdata->cdev, desc.name); > + if (IS_ERR(drvdata->cdev.dev)) { > + ret = PTR_ERR(drvdata->cdev.dev); > + goto err_delete_cdev; > + } else > pm_runtime_put(&adev->dev); > + > + return 0; > + > +err_delete_cdev: > + cdev_del(&drvdata->cdev.cdev); > +err_free_tmc_ida: > + ida_simple_remove(&tmc_ida, MINOR(devt)); > +err_coresight_unregister: > + coresight_unregister(drvdata->csdev); > out: > return ret; > } > @@ -583,13 +613,11 @@ static void tmc_shutdown(struct amba_device *adev) > static void tmc_remove(struct amba_device *adev) > { > struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + struct device *dev = drvdata->cdev.dev; > > - /* > - * Since misc_open() holds a refcount on the f_ops, which is > - * etb fops in this case, device is there until last file > - * handler to this device is closed. > - */ > - misc_deregister(&drvdata->miscdev); > + ida_simple_remove(&tmc_ida, MINOR(dev->devt)); > + device_destroy(tmc_class, dev->devt); > + cdev_del(&drvdata->cdev.cdev); > coresight_unregister(drvdata->csdev); > } > > @@ -618,7 +646,42 @@ static struct amba_driver tmc_driver = { > .id_table = tmc_ids, > }; > > -module_amba_driver(tmc_driver); > +static int __init tmc_init(void) > +{ > + int ret; > + > + ret = alloc_chrdev_region(&tmc_major, 0, TMC_DEV_MAX, "tmc"); > + if (ret < 0) { > + pr_err("tmc: failed to allocate char dev region\n"); > + return ret; > + } > + > + tmc_class = class_create(THIS_MODULE, "tmc"); > + if (IS_ERR(tmc_class)) { > + pr_err("tmc: failed to create class\n"); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > + return PTR_ERR(tmc_class); > + } > + > + ret = amba_driver_register(&tmc_driver); > + if (ret) { > + pr_err("tmc: error registering amba driver\n"); > + class_destroy(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > + } > + > + return ret; > +} > + > +static void __exit tmc_exit(void) > +{ > + amba_driver_unregister(&tmc_driver); > + class_destroy(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > +} > + > +module_init(tmc_init); > +module_exit(tmc_exit); > > MODULE_AUTHOR("Pratik Patel <pratikp(a)codeaurora.org>"); > MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver"); > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index 6bec20a392b3..11441c6b9c26 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -7,9 +7,9 @@ > #ifndef _CORESIGHT_TMC_H > #define _CORESIGHT_TMC_H > > +#include <linux/cdev.h> > #include <linux/dma-mapping.h> > #include <linux/idr.h> > -#include <linux/miscdevice.h> > #include <linux/mutex.h> > #include <linux/refcount.h> > > @@ -163,11 +163,16 @@ struct etr_buf { > void *private; > }; > > +struct tmc_cdev { > + struct cdev cdev; > + struct device *dev; > +}; > + > /** > * struct tmc_drvdata - specifics associated to an TMC component > * @base: memory mapped base address for this component. > * @csdev: component vitals needed by the framework. > - * @miscdev: specifics to handle "/dev/xyz.tmc" entry. > + * @tmc_cdev: specifics to handle "/dev/xyz.tmc" entry. > * @spinlock: only one at a time pls. > * @pid: Process ID of the process being monitored by the session > * that is using this component. > @@ -191,7 +196,7 @@ struct etr_buf { > struct tmc_drvdata { > void __iomem *base; > struct coresight_device *csdev; > - struct miscdevice miscdev; > + struct tmc_cdev cdev; > spinlock_t spinlock; > pid_t pid; > bool reading; > -- > 2.27.0 >

2 years, 9 months

Re: [PATCH V2 0/2] coresight: trbe: Update existing errata for Cortex-X2

by Suzuki K Poulose

On 24/01/2022 03:15, Anshuman Khandual wrote: > Errata ARM64_ERRATUM_[2119858|2224489] also affect some Cortex-X2 ranges as > well. This series updates the errata definition and detection as required. > This series applies on v5.17-rc1. > > Relevant identification document can be found here. > > https://developer.arm.com/documentation/101803/0200/AArch64-system-register… > AArch64-identification-register-summary/MIDR-EL1--Main-ID-Register > > Relevant errata document can be found here. > > https://developer.arm.com/documentation/SDEN1775100 > > Cc: Catalin Marinas <catalin.marinas(a)arm.com> > Cc: Will Deacon <will(a)kernel.org> > Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> > Cc: Suzuki Poulose <suzuki.poulose(a)arm.com> > Cc: coresight(a)lists.linaro.org > Cc: linux-arm-kernel(a)lists.infradead.org > Cc: linux-kernel(a)vger.kernel.org > For the series: Reviewed-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>

2 years, 9 months

[PATCH v2 0/7] pid: Introduce helper task_is_in_root_ns()

by Leo Yan

The kernel uses open code to check if a process is in root PID namespace or not in several places. Suggested by Suzuki, this patch set is to create a helper function task_is_in_init_pid_ns() to replace open code. This patch set has been applied on the mainline kernel and built for Arm64 kernel with enabling all relevant modules. Changes from v1: * Renamed helper function from task_is_in_root_ns() to task_is_in_init_pid_ns(). (Leon Romanovsky) * Improved patches' commit logs for more neat. Leo Yan (7): pid: Introduce helper task_is_in_init_pid_ns() coresight: etm3x: Use task_is_in_init_pid_ns() coresight: etm4x: Use task_is_in_init_pid_ns() connector/cn_proc: Use task_is_in_init_pid_ns() coda: Use task_is_in_init_pid_ns() audit: Use task_is_in_init_pid_ns() taskstats: Use task_is_in_init_pid_ns() drivers/connector/cn_proc.c | 2 +- drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 8 ++++---- drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++---- fs/coda/inode.c | 2 +- fs/coda/psdev.c | 2 +- include/linux/pid_namespace.h | 5 +++++ kernel/audit.c | 2 +- kernel/taskstats.c | 2 +- 8 files changed, 18 insertions(+), 13 deletions(-) -- 2.25.1

2 years, 9 months

Re: [PATCH v2 2/9] Coresight: Add coresight TPDM source driver

by Mathieu Poirier

Hey Jinlong, On Fri, Jan 21, 2022 at 10:01:47PM +0800, Jinlong Mao wrote: > Hi Mathieu, > > Good Day. > > On 12/17/2021 3:02 AM, Mathieu Poirier wrote: > > [...] > > > > > > > > + > > > > > > +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) > > > > > > +{ > > > > > > + static int traceid = TPDM_TRACE_ID_START; > > > > > > + > > > > > > + drvdata->traceid = traceid++; > > > > > > +} > > > > > I have been specific on how to properly do this in the last revision. Given the > > > > > above about the MAINTAINERS file, I am not sure that I will continue reviewing this set. > > > > > > > > > > There is also no need to rush another revision as I won't have the bandwidth to > > > > > process it before the holidays. > > > > > > > > > > Thanks, > > > > > Mathieu > > > > Hi Mathieu, > > > > > > > > Sorry, not addressed your previous comments here. > > > > > > > > For the trace id, each coresight component has 7 bits to store the trace > > > > id. So the trace id should be from 1 to 127 as 0 is invalid. > > > IDs 0x70 - 0x7F (`112 - 127 ) are reserved - see the ARM Coresight > > > Architecture specification v3.0 > > > > > Correct > > > > > > Apart from TPDMs/STM/ETMs, we also have other coresight components in > > > > our internal device. About 80 ids are already used. > > > > > > > > Some components have fixed trace id in HW. If we use functions below to > > > > count the trace id, there will be conflict to other components. > > > > > > > > Can we use 1-15 for etm trace ids and 16 - 127 for other coresight > > > > components ? And handle trace ids in its' own driver ? > > > > > > > This will limit systems to 15 cores - some have more! > > > > > Correct > > > > > > static inline int coresight_get_system_trace_id(int id) > > > > { > > > > /* Start system IDs above the highest per CPU trace ID. */ > > > > return coresigth_get_trace_id(cpumask_last(cpu_possible_mask) + 1); > > > > } > > Looking at my own suggestion again this won't work since it returns the same traceID > > when called multiple times. > > > > For this patchset and _without_ taking into account internal devices that have > > their traceID set in HW: > > > > 1. Define a bitmask that is 7 bit wide. > Should it be a 128 bit wide bitmask (0--127)? Yes, you are correct. > > 2. By default, set bits under 0x10 and between 0x70 - 0x7F. > > 3. In coresight_get_system_trace_id(), drop the @id parameter and allocate the > > first available bit after cpumask_last(cpu_possible_mask) + 1. > > Should it allocate the first available bit after (cpumask_last(cpu_possible_mask) *2 ) + 0x10 ? > Return the first zero bit position as the trace id and set the bit. I need to clarify something with Mike on this - I will get back to you on Monday. > > > 4. Define a new function called coresight_put_system_trace_id(int id) that > > clears the bit in the mask corresponding to @id. > > > > For now that should work. > > > > > > static inline int coresight_get_trace_id(int cpu) > > > > { > > > > /* > > > > * A trace ID of value 0 is invalid, so let's start at some > > > > * random value that fits in 7 bits and go from there. Since > > > > * the common convention is to have data trace IDs be I(N) + 1, > > > > * set instruction trace IDs as a function of the CPU number. > > > > */ > > > > return (CORESIGHT_ETM_PMU_SEED + (cpu * 2)); > > > > } > > > > > > > This fixed relationship between cpu and trace ID is used in the perf > > > tooling to populate the elements in the perf.data file to correctly > > > allow association between CPU and trace data, and thus allow correct > > > trace decode. > > TraceIDs associated to CPUs are communicated to the perf tooling by way of the > > perf header - theoretically we should be able to change the allocation scheme > > without impacting the decoding process. > > > > > It should be possible to create another more dynamic mapping scheme - > > > but this must include a way to support the perf requirements too. > > > > > TraceIDs have been a lurking problem for as long as the subsystem has existed. > > For now what I have suggested above should be sufficient to provide an > > in-between solution that doesn't hold back this patchset. > > > > That being said, we need to start thinking about the best way to do this. I > > will put a patchset together in the new year that aims in that direction. > > > > > Regards > > > > > > Mike > > > > > > > Thanks > > > > > > > > Jinlong Mao > > > > > > > > > > + > > > > > > +static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > > > > > > +{ > > > > > > + struct device *dev = &adev->dev; > > > > > > + struct coresight_platform_data *pdata; > > > > > > + struct tpdm_drvdata *drvdata; > > > > > > + struct coresight_desc desc = { 0 }; > > > > > > + > > > > > > + desc.name = coresight_alloc_device_name(&tpdm_devs, dev); > > > > > > + if (!desc.name) > > > > > > + return -ENOMEM; > > > > > > + pdata = coresight_get_platform_data(dev); > > > > > > + if (IS_ERR(pdata)) > > > > > > + return PTR_ERR(pdata); > > > > > > + adev->dev.platform_data = pdata; > > > > > > + > > > > > > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > > > > > > + if (!drvdata) > > > > > > + return -ENOMEM; > > > > > > + drvdata->dev = &adev->dev; > > > > > > + dev_set_drvdata(dev, drvdata); > > > > > > + > > > > > > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > > > > > > + if (!drvdata->base) > > > > > > + return -ENOMEM; > > > > > > + > > > > > > + mutex_init(&drvdata->lock); > > > > > > + > > > > > > + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > > > > > > + desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SYS; > > > > > > + desc.ops = &tpdm_cs_ops; > > > > > > + desc.pdata = adev->dev.platform_data; > > > > > > + desc.dev = &adev->dev; > > > > > > + drvdata->csdev = coresight_register(&desc); > > > > > > + if (IS_ERR(drvdata->csdev)) > > > > > > + return PTR_ERR(drvdata->csdev); > > > > > > + > > > > > > + tpdm_init_default_data(drvdata); > > > > > > + pm_runtime_put(&adev->dev); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static void __exit tpdm_remove(struct amba_device *adev) > > > > > > +{ > > > > > > + struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > > > > > + > > > > > > + coresight_unregister(drvdata->csdev); > > > > > > +} > > > > > > + > > > > > > +static struct amba_id tpdm_ids[] = { > > > > > > + { > > > > > > + .id = 0x000f0e00, > > > > > > + .mask = 0x000fff00, > > > > > > + }, > > > > > > + { 0, 0}, > > > > > > +}; > > > > > > + > > > > > > +static struct amba_driver tpdm_driver = { > > > > > > + .drv = { > > > > > > + .name = "coresight-tpdm", > > > > > > + .owner = THIS_MODULE, > > > > > > + .suppress_bind_attrs = true, > > > > > > + }, > > > > > > + .probe = tpdm_probe, > > > > > > + .id_table = tpdm_ids, > > > > > > +}; > > > > > > + > > > > > > +module_amba_driver(tpdm_driver); > > > > > > + > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver"); > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > > > > > > new file mode 100644 > > > > > > index 000000000000..980ae90ff1c8 > > > > > > --- /dev/null > > > > > > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > > > > > > @@ -0,0 +1,31 @@ > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > +/* > > > > > > + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > > > > > > + */ > > > > > > + > > > > > > +#ifndef _CORESIGHT_CORESIGHT_TPDM_H > > > > > > +#define _CORESIGHT_CORESIGHT_TPDM_H > > > > > > + > > > > > > +/* Default value of the traceid */ > > > > > > +#define TPDM_TRACE_ID_START 128 > > > > > > + > > > > > > +/** > > > > > > + * struct tpdm_drvdata - specifics associated to an TPDM component > > > > > > + * @base: memory mapped base address for this component. > > > > > > + * @dev: The device entity associated to this component. > > > > > > + * @csdev: component vitals needed by the framework. > > > > > > + * @lock: lock for the enable value. > > > > > > + * @enable: enable status of the component. > > > > > > + * @traceid: value of the current ID for this component. > > > > > > + */ > > > > > > + > > > > > > +struct tpdm_drvdata { > > > > > > + void __iomem *base; > > > > > > + struct device *dev; > > > > > > + struct coresight_device *csdev; > > > > > > + struct mutex lock; > > > > > > + bool enable; > > > > > > + int traceid; > > > > > > +}; > > > > > > + > > > > > > +#endif /* _CORESIGHT_CORESIGHT_TPDM_H */ > > > > > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > > > > > > index 93a2922b7653..e48d463be63b 100644 > > > > > > --- a/include/linux/coresight.h > > > > > > +++ b/include/linux/coresight.h > > > > > > @@ -65,6 +65,7 @@ enum coresight_dev_subtype_source { > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_BUS, > > > > > > CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE, > > > > > > + CORESIGHT_DEV_SUBTYPE_SOURCE_SYS, > > > > > > }; > > > > > > > > > > > > enum coresight_dev_subtype_helper { > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > > -- > > > Mike Leach > > > Principal Engineer, ARM Ltd. > > > Manchester Design Centre. UK

2 years, 9 months

[PATCH 0/1] coresight: Fix TRCCONFIGR.QE sysfs interface

by James Clark

I was working on refactoring some of the magic numbers for the register accesses and I saw this issue with programming one of the values. I don't have any evidence that someone encountered it not working, probably there is no hardware where QSUPP==0b10, or it just didn't get used yet. But the issue is that this silently writes a reserved value even if the user provided one was correct so it might ruin someones day if it ever comes up. It's a small change so I didn't see the harm in fixing it. Applies to coresight/next f9809d565135 James Clark (1): coresight: Fix TRCCONFIGR.QE sysfs interface drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.17.1

2 years, 9 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight January 2022