| Message ID | 20211217034208.64290-1-jkchen@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,V2] coresight: change tmc from misc device to cdev device | expand |
Hi Jay, On Fri, Dec 17, 2021 at 11:42:08AM +0800, Jay Chen wrote: > From: root <root@localhost.localdomain> Please use your company's email address rather than using an invalid email address. > For coresight in the server scenario, if there are too many > funnel (about 130) and tmc (about 130) peripherals, the error > of insufficient device numbers will occur by misc device Incomplete commit log ... For a better practice, it's deserve to write a high quality commit log which is helpful for maintainers to understand your patch, at the end, it also can accelerate your patch's merging. Let me quote a suggestion for the commit log format from an old reply [1]: ... Please use the customary changelog style we use in the kernel: " Current code does (A), this has a problem when (B). We can improve this doing (C), because (D)." [1] https://lkml.org/lkml/2013/11/11/146 > Signed-off-by: root <root@localhost.localdomain> > --- > .../hwtracing/coresight/coresight-tmc-core.c | 84 +++++++++++++++---- > drivers/hwtracing/coresight/coresight-tmc.h | 10 ++- > 2 files changed, 75 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index d0276af82494..03b114c0616a 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > +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; > @@ -147,7 +152,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 +184,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 +205,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) > @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) > > static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > { > - int ret = 0; > + int ret = 0, tmc_num = 0; > u32 devid; > + dev_t devt; > void __iomem *base; > struct device *dev = &adev->dev; > struct coresight_platform_data *pdata = NULL; > @@ -546,14 +552,26 @@ 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); > - if (ret) > + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + > + etr_devs.nr_idx; I am struggling to understand this line code, but looks to me this is right. The system might have both ETR and ETF components, so we need to calculate index for all sinks. > + > + cdev_init(&drvdata->cdev.cdev, &tmc_fops); > + drvdata->cdev.cdev.owner = THIS_MODULE; > + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); > + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); > + if (ret) { > coresight_unregister(drvdata->csdev); > - else > + goto out; > + } > + > + 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); > + cdev_del(&drvdata->cdev.cdev); > + coresight_unregister(drvdata->csdev); > + } else > pm_runtime_put(&adev->dev); > + > out: > return ret; > } > @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) > { > struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->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); > + device_destroy(tmc_class, drvdata->cdev.dev->devt); > + cdev_del(&drvdata->cdev.cdev); > coresight_unregister(drvdata->csdev); > } > > @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); > + ret = PTR_ERR(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > + return ret; > + } > + > + 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); Should invoke tmc_init() in an ealier phase so it can prepare device class prior to the TMC devices registration? In other words, I think here should be: subsys_initcall(tmc_init); Thanks, Leo > +module_exit(tmc_exit); > > MODULE_AUTHOR("Pratik Patel <pratikp@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..b65ac363f9e4 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -7,6 +7,7 @@ > #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> > @@ -163,11 +164,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 +197,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 >
Good day Jay, On Fri, Dec 17, 2021 at 11:42:08AM +0800, Jay Chen wrote: > From: root <root@localhost.localdomain> > > For coresight in the server scenario, if there are too many > funnel (about 130) and tmc (about 130) peripherals, the error > of insufficient device numbers will occur by misc device > Why is the changelog mention funnels when they have nothing to do with misc devices? > Signed-off-by: root <root@localhost.localdomain> > --- > .../hwtracing/coresight/coresight-tmc-core.c | 84 +++++++++++++++---- > drivers/hwtracing/coresight/coresight-tmc.h | 10 ++- > 2 files changed, 75 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index d0276af82494..03b114c0616a 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > +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; > @@ -147,7 +152,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 +184,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 +205,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) > @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) > > static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > { > - int ret = 0; > + int ret = 0, tmc_num = 0; > u32 devid; > + dev_t devt; > void __iomem *base; > struct device *dev = &adev->dev; > struct coresight_platform_data *pdata = NULL; > @@ -546,14 +552,26 @@ 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); > - if (ret) > + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + > + etr_devs.nr_idx; Humm... This is turning out to be a lot worse than expected. Please use an IDA, the same way the rpmsg_char driver does. > + > + cdev_init(&drvdata->cdev.cdev, &tmc_fops); > + drvdata->cdev.cdev.owner = THIS_MODULE; > + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); > + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); > + if (ret) { > coresight_unregister(drvdata->csdev); > - else > + goto out; > + } > + > + 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); > + cdev_del(&drvdata->cdev.cdev); > + coresight_unregister(drvdata->csdev); > + } else > pm_runtime_put(&adev->dev); > + > out: > return ret; > } > @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) > { > struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->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); > + device_destroy(tmc_class, drvdata->cdev.dev->devt); > + cdev_del(&drvdata->cdev.cdev); > coresight_unregister(drvdata->csdev); > } > > @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); > + ret = PTR_ERR(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > + return ret; > + } > + > + 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@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..b65ac363f9e4 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -7,6 +7,7 @@ > #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> > @@ -163,11 +164,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 +197,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 >
On Fri, Dec 17, 2021 at 03:54:27PM +0800, Leo Yan wrote: > Hi Jay, > > On Fri, Dec 17, 2021 at 11:42:08AM +0800, Jay Chen wrote: > > From: root <root@localhost.localdomain> > > Please use your company's email address rather than using an invalid > email address. > > > For coresight in the server scenario, if there are too many > > funnel (about 130) and tmc (about 130) peripherals, the error > > of insufficient device numbers will occur by misc device > > Incomplete commit log ... > > For a better practice, it's deserve to write a high quality commit log > which is helpful for maintainers to understand your patch, at the end, > it also can accelerate your patch's merging. Let me quote a suggestion > for the commit log format from an old reply [1]: > > ... Please use the customary changelog style we use in the kernel: > > " Current code does (A), this has a problem when (B). > We can improve this doing (C), because (D)." > > [1] https://lkml.org/lkml/2013/11/11/146 > > > Signed-off-by: root <root@localhost.localdomain> > > > --- > > .../hwtracing/coresight/coresight-tmc-core.c | 84 +++++++++++++++---- > > drivers/hwtracing/coresight/coresight-tmc.h | 10 ++- > > 2 files changed, 75 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > > index d0276af82494..03b114c0616a 100644 > > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > > @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > > > +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; > > @@ -147,7 +152,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 +184,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 +205,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) > > @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) > > > > static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > > { > > - int ret = 0; > > + int ret = 0, tmc_num = 0; > > u32 devid; > > + dev_t devt; > > void __iomem *base; > > struct device *dev = &adev->dev; > > struct coresight_platform_data *pdata = NULL; > > @@ -546,14 +552,26 @@ 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); > > - if (ret) > > + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + > > + etr_devs.nr_idx; > > I am struggling to understand this line code, but looks to me this is > right. The system might have both ETR and ETF components, so we > need to calculate index for all sinks. > > > + > > + cdev_init(&drvdata->cdev.cdev, &tmc_fops); > > + drvdata->cdev.cdev.owner = THIS_MODULE; > > + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); > > + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); > > + if (ret) { > > coresight_unregister(drvdata->csdev); > > - else > > + goto out; > > + } > > + > > + 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); > > + cdev_del(&drvdata->cdev.cdev); > > + coresight_unregister(drvdata->csdev); > > + } else > > pm_runtime_put(&adev->dev); > > + > > out: > > return ret; > > } > > @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) > > { > > struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->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); > > + device_destroy(tmc_class, drvdata->cdev.dev->devt); > > + cdev_del(&drvdata->cdev.cdev); > > coresight_unregister(drvdata->csdev); > > } > > > > @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); > > + ret = PTR_ERR(tmc_class); > > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > > + return ret; > > + } > > + > > + 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); > > Should invoke tmc_init() in an ealier phase so it can prepare device class > prior to the TMC devices registration? In other words, I think here > should be: > > subsys_initcall(tmc_init); TMC devices won't be registered until a TMC driver is registered, which happens after the creation of the class. Anything I am missing? > > Thanks, > Leo > > > +module_exit(tmc_exit); > > > > MODULE_AUTHOR("Pratik Patel <pratikp@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..b65ac363f9e4 100644 > > --- a/drivers/hwtracing/coresight/coresight-tmc.h > > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > > @@ -7,6 +7,7 @@ > > #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> > > @@ -163,11 +164,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 +197,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 > >
On Fri, Dec 17, 2021 at 10:18:59AM -0700, Mathieu Poirier wrote: [...] > > > @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); > > > + ret = PTR_ERR(tmc_class); > > > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > > > + return ret; > > > + } > > > + > > > + 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; > > > +} [...] > > > +module_init(tmc_init); > > > > Should invoke tmc_init() in an ealier phase so it can prepare device class > > prior to the TMC devices registration? In other words, I think here > > should be: > > > > subsys_initcall(tmc_init); > > TMC devices won't be registered until a TMC driver is registered, which happens > after the creation of the class. Anything I am missing? No, I introduced noise and sorry for that. The race condition between creating TMC class (subsystem) and TMC driver probing has been handled well by this patch. It removes "module_amba_driver(tmc_driver)", then in tmc_init(), uses amba_driver_register() to register TMC driver. So that can ensure the TMC class has been created prior to TMC driver registeration and TMC driver probing. Please ignore my comment for using subsys_initcall(). Thanks, Leo
Hi Leo 在 2021/12/17 15:54, Leo Yan 写道: > Hi Jay, > > On Fri, Dec 17, 2021 at 11:42:08AM +0800, Jay Chen wrote: >> From: root <root@localhost.localdomain> > Please use your company's email address rather than using an invalid > email address. > >> For coresight in the server scenario, if there are too many >> funnel (about 130) and tmc (about 130) peripherals, the error >> of insufficient device numbers will occur by misc device > Incomplete commit log ... > > For a better practice, it's deserve to write a high quality commit log > which is helpful for maintainers to understand your patch, at the end, > it also can accelerate your patch's merging. Let me quote a suggestion > for the commit log format from an old reply [1]: > > ... Please use the customary changelog style we use in the kernel: > > " Current code does (A), this has a problem when (B). > We can improve this doing (C), because (D)." > > [1] https://lkml.org/lkml/2013/11/11/146 Thank you for your suggestion. I will describe it carefully >> Signed-off-by: root <root@localhost.localdomain> >> --- >> .../hwtracing/coresight/coresight-tmc-core.c | 84 +++++++++++++++---- >> drivers/hwtracing/coresight/coresight-tmc.h | 10 ++- >> 2 files changed, 75 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c >> index d0276af82494..03b114c0616a 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c >> @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); >> DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); >> DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); >> >> +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; >> @@ -147,7 +152,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 +184,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 +205,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) >> @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) >> >> static int tmc_probe(struct amba_device *adev, const struct amba_id *id) >> { >> - int ret = 0; >> + int ret = 0, tmc_num = 0; >> u32 devid; >> + dev_t devt; >> void __iomem *base; >> struct device *dev = &adev->dev; >> struct coresight_platform_data *pdata = NULL; >> @@ -546,14 +552,26 @@ 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); >> - if (ret) >> + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + >> + etr_devs.nr_idx; > I am struggling to understand this line code, but looks to me this is > right. The system might have both ETR and ETF components, so we > need to calculate index for all sinks. Yeah, my system have both ETR and ETF components; >> + >> + cdev_init(&drvdata->cdev.cdev, &tmc_fops); >> + drvdata->cdev.cdev.owner = THIS_MODULE; >> + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); >> + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); >> + if (ret) { >> coresight_unregister(drvdata->csdev); >> - else >> + goto out; >> + } >> + >> + 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); >> + cdev_del(&drvdata->cdev.cdev); >> + coresight_unregister(drvdata->csdev); >> + } else >> pm_runtime_put(&adev->dev); >> + >> out: >> return ret; >> } >> @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) >> { >> struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->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); >> + device_destroy(tmc_class, drvdata->cdev.dev->devt); >> + cdev_del(&drvdata->cdev.cdev); >> coresight_unregister(drvdata->csdev); >> } >> >> @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); >> + ret = PTR_ERR(tmc_class); >> + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); >> + return ret; >> + } >> + >> + 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); > Should invoke tmc_init() in an ealier phase so it can prepare device class > prior to the TMC devices registration? In other words, I think here > should be: > > subsys_initcall(tmc_init); > > Thanks, > Leo Your suggestion is very good. I will revise it according to your suggestion; >> +module_exit(tmc_exit); >> MODULE_AUTHOR("Pratik Patel <pratikp@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..b65ac363f9e4 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc.h >> +++ b/drivers/hwtracing/coresight/coresight-tmc.h >> @@ -7,6 +7,7 @@ >> #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> >> @@ -163,11 +164,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 +197,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 >> Tks Jay
Hi Jay, On Mon, Dec 20, 2021 at 10:32:21AM +0800, Jiankang Chen wrote: [...] > > > +module_init(tmc_init); > > Should invoke tmc_init() in an ealier phase so it can prepare device class > > prior to the TMC devices registration? In other words, I think here > > should be: > > > > subsys_initcall(tmc_init); > > > Your suggestion is very good. I will revise it according to your suggestion; As I clarified for Mathieu's concerning in another reply, please ignore this comment. Thanks, Leo
Hi Leo 在 2021/12/20 10:37, Leo Yan 写道: > Hi Jay, > > On Mon, Dec 20, 2021 at 10:32:21AM +0800, Jiankang Chen wrote: > > [...] > >>>> +module_init(tmc_init); >>> Should invoke tmc_init() in an ealier phase so it can prepare device class >>> prior to the TMC devices registration? In other words, I think here >>> should be: >>> >>> subsys_initcall(tmc_init); >>> >> Your suggestion is very good. I will revise it according to your suggestion; > As I clarified for Mathieu's concerning in another reply, please ignore > this comment. > > Thanks, > Leo Ok, Tks Jay
Hi Mathieu 在 2021/12/18 01:15, Mathieu Poirier 写道: > Good day Jay, > > On Fri, Dec 17, 2021 at 11:42:08AM +0800, Jay Chen wrote: >> From: root <root@localhost.localdomain> >> >> For coresight in the server scenario, if there are too many >> funnel (about 130) and tmc (about 130) peripherals, the error >> of insufficient device numbers will occur by misc device >> > Why is the changelog mention funnels when they have nothing to do with misc > devices? > > > This changelog briefly explains the architecture of a certain place, and I will give a more detailed commit log later >> Signed-off-by: root <root@localhost.localdomain> >> --- >> .../hwtracing/coresight/coresight-tmc-core.c | 84 +++++++++++++++---- >> drivers/hwtracing/coresight/coresight-tmc.h | 10 ++- >> 2 files changed, 75 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c >> index d0276af82494..03b114c0616a 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c >> @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); >> DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); >> DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); >> >> +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; >> @@ -147,7 +152,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 +184,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 +205,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) >> @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) >> >> static int tmc_probe(struct amba_device *adev, const struct amba_id *id) >> { >> - int ret = 0; >> + int ret = 0, tmc_num = 0; >> u32 devid; >> + dev_t devt; >> void __iomem *base; >> struct device *dev = &adev->dev; >> struct coresight_platform_data *pdata = NULL; >> @@ -546,14 +552,26 @@ 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); >> - if (ret) >> + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + >> + etr_devs.nr_idx; > Humm... This is turning out to be a lot worse than expected. Please use an IDA, > the same way the rpmsg_char driver does. ok, I will change it; >> + >> + cdev_init(&drvdata->cdev.cdev, &tmc_fops); >> + drvdata->cdev.cdev.owner = THIS_MODULE; >> + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); >> + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); >> + if (ret) { >> coresight_unregister(drvdata->csdev); >> - else >> + goto out; >> + } >> + >> + 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); >> + cdev_del(&drvdata->cdev.cdev); >> + coresight_unregister(drvdata->csdev); >> + } else >> pm_runtime_put(&adev->dev); >> + >> out: >> return ret; >> } >> @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) >> { >> struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->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); >> + device_destroy(tmc_class, drvdata->cdev.dev->devt); >> + cdev_del(&drvdata->cdev.cdev); >> coresight_unregister(drvdata->csdev); >> } >> >> @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); >> + ret = PTR_ERR(tmc_class); >> + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); >> + return ret; >> + } >> + >> + 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@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..b65ac363f9e4 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc.h >> +++ b/drivers/hwtracing/coresight/coresight-tmc.h >> @@ -7,6 +7,7 @@ >> #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> >> @@ -163,11 +164,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 +197,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 >> Tks Jay
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index d0276af82494..03b114c0616a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); +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; @@ -147,7 +152,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 +184,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 +205,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) @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) static int tmc_probe(struct amba_device *adev, const struct amba_id *id) { - int ret = 0; + int ret = 0, tmc_num = 0; u32 devid; + dev_t devt; void __iomem *base; struct device *dev = &adev->dev; struct coresight_platform_data *pdata = NULL; @@ -546,14 +552,26 @@ 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); - if (ret) + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + + etr_devs.nr_idx; + + cdev_init(&drvdata->cdev.cdev, &tmc_fops); + drvdata->cdev.cdev.owner = THIS_MODULE; + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); + if (ret) { coresight_unregister(drvdata->csdev); - else + goto out; + } + + 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); + cdev_del(&drvdata->cdev.cdev); + coresight_unregister(drvdata->csdev); + } else pm_runtime_put(&adev->dev); + out: return ret; } @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) { struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->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); + device_destroy(tmc_class, drvdata->cdev.dev->devt); + cdev_del(&drvdata->cdev.cdev); coresight_unregister(drvdata->csdev); } @@ -618,7 +632,43 @@ 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("failed to create tmc class\n"); + ret = PTR_ERR(tmc_class); + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); + return ret; + } + + 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@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..b65ac363f9e4 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -7,6 +7,7 @@ #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> @@ -163,11 +164,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 +197,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;