Re: [RFC PATCH] Change coresight tmc from misc device to cdev device

Hi Jay,

On Wed, Dec 01, 2021 at 03:09:42PM +0800, Jay Chen wrote:

For coresight in the server scenario, if there are too many cpu cores and too many tmc peripherals, the error of insufficient device numbers will occur by misc device

There really is more than 128 miscdevice allocated in your system? How may TMC do you have?

Signed-off-by: Jay Chen jkchen@linux.alibaba.com

drivers/hwtracing/coresight/coresight-core.c | 53 +++++++++++++++++++ .../hwtracing/coresight/coresight-tmc-core.c | 13 ++--- drivers/hwtracing/coresight/coresight-tmc.h | 4 +- include/linux/coresight.h | 23 ++++++++ 4 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 8a18c71df37a..b0cca3060f4e 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -17,6 +17,7 @@ #include <linux/coresight.h> #include <linux/of_platform.h> #include <linux/delay.h> +#include <linux/cdev.h> #include <linux/pm_runtime.h> #include "coresight-etm-perf.h" @@ -1567,6 +1568,58 @@ void coresight_release_platform_data(struct coresight_device *csdev, coresight_remove_conns_sysfs_group(csdev); } +int coresight_cdev_register(struct coresight_cdev *cdev, const struct file_operations *fops,

• 	    const char *name)
  

+{

• struct device *device;
• if (alloc_chrdev_region(&cdev->devno, 0, 1, name)) {

• pr_err("failed to create the coresight cdev region\n");
  

• return -EFAULT;
  

• }
• cdev->class = class_create(THIS_MODULE, name);
• if (IS_ERR(cdev->class)) {

• pr_err("failed to create the coresight class\n");
  

• goto coresight_unregister_region;
  

• }

From the above a new class is created for every TMC, even if they are all the

same. A better way to do this would be to create a single TMC class with a new cdev for every new TMC.

To do this alloc_chrdev_region() and class_create() have to be called when the module is added to the system. Have a look at funnel_init() on how to refactor the TMC core driver with an init function and rpmsg_chrdev_init() for the chrdev region allocation and the class creation.

From there every time a TMC is probe cdev_init() and cdev_add() can be called -

look at rpmsg_chrdev_probe() for an example. Instead of using an IDA to pick out a minor number you should be able to use dev_list->nr_idx - 1.

Thanks, Mathieu

• cdev_init(&cdev->cdev, fops);
• if (cdev_add(&cdev->cdev, cdev->devno, 1)) {

• pr_err("failed to add the coresight cdev\n");
  

• goto coresight_free_class;
  

• }
• device = device_create(cdev->class, NULL, cdev->devno,

• 	       cdev, name);
  

• if (IS_ERR(device)) {

• pr_err("failed to create device for coresight\n");
  

• goto coresight_del_cdev;
  

• }
• return 0;

+coresight_del_cdev:

• cdev_del(&cdev->cdev);

+coresight_free_class:

• class_destroy(cdev->class);

+coresight_unregister_region:

• unregister_chrdev_region(cdev->devno, 1);
• return -EFAULT;

+} +EXPORT_SYMBOL_GPL(coresight_cdev_register);

+int coresight_cdev_unregister(struct coresight_cdev *cdev) +{

• device_destroy(cdev->class, cdev->devno);
• cdev_del(&cdev->cdev);
• class_destroy(cdev->class);
• unregister_chrdev_region(cdev->devno, 1);
• return 0;

+} +EXPORT_SYMBOL_GPL(coresight_cdev_unregister);

struct coresight_device *coresight_register(struct coresight_desc *desc) { int ret; diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 74c6323d4d6a..c4aae8ff65eb 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -147,7 +147,7 @@ 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_drvdata, cdev);
  

ret = tmc_read_prepare(drvdata); if (ret) @@ -179,7 +179,7 @@ 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_drvdata, cdev);
  

  actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp); if (actual <= 0) return 0;

@@ -200,7 +200,7 @@ 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_drvdata, cdev);
  

ret = tmc_read_unprepare(drvdata); if (ret) @@ -529,10 +529,7 @@ 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 = coresight_cdev_register(&drvdata->cdev, &tmc_fops, desc.name); if (ret) coresight_unregister(drvdata->csdev); else

@@ -572,7 +569,7 @@ static void tmc_remove(struct amba_device *adev) * etb fops in this case, device is there until last file * handler to this device is closed. */

• misc_deregister(&drvdata->miscdev);

• coresight_cdev_unregister(&drvdata->cdev); coresight_unregister(drvdata->csdev);

} diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index b91ec7dde7bc..c37e2ed1cb9d 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -166,7 +166,7 @@ struct etr_buf {

• 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.

• • @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.
    

@@ -188,7 +188,7 @@ struct etr_buf { struct tmc_drvdata { void __iomem *base; struct coresight_device *csdev;

• struct miscdevice miscdev;

• struct coresight_cdev cdev; spinlock_t spinlock; pid_t pid; bool reading;

diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 93a2922b7653..aebe8b8327ad 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -9,6 +9,7 @@ #include <linux/device.h> #include <linux/io.h> #include <linux/perf_event.h> +#include <linux/cdev.h> #include <linux/sched.h> /* Peripheral id registers (0xFD0-0xFEC) */ @@ -252,6 +253,12 @@ struct coresight_device { void *active_cscfg_ctxt; }; +struct coresight_cdev {

• dev_t devno;
• struct class *class;
• struct cdev cdev;

+};

/*

• coresight_dev_list - Mapping for devices to "name" index for device
• names.

@@ -506,6 +513,10 @@ void coresight_relaxed_write64(struct coresight_device *csdev, u64 val, u32 offset); void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset); +int coresight_cdev_register(struct coresight_cdev *cdev, const struct file_operations *fops,

• 	const char *name);
  

+int coresight_cdev_unregister(struct coresight_cdev *cdev); #else static inline struct coresight_device * coresight_register(struct coresight_desc *desc) { return NULL; } @@ -581,6 +592,18 @@ static inline void coresight_write64(struct coresight_device *csdev, u64 val, u3 { } +static inline int coresight_cdev_register(struct coresight_cdev *cdev,

• 			  const struct file_operations *fops,
  

• 			  const char *name)
  

+{

• return 0;

+}

+static inline int coresight_cdev_unregister(struct coresight_cdev *cdev) +{

• return 0;

+}

#endif /* IS_ENABLED(CONFIG_CORESIGHT) */ extern int coresight_get_cpu(struct device *dev); -- 2.27.0