2023-11-22 08:03:57

by Gangurde, Abhijit

[permalink] [raw]
Subject: [PATCH 2/2] cdx: Create resource debugfs file for cdx device

resource debugfs file contains host addresses of CDX device resources.
Each line of the resource file describe a region with start, end and
flag fields.

Signed-off-by: Abhijit Gangurde <[email protected]>
---
drivers/cdx/cdx.c | 42 ++++++++++++++++++++++++++++++++++++-
include/linux/cdx/cdx_bus.h | 2 ++
2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 4edf64f9e98d..e01376272ed3 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -65,6 +65,7 @@
#include <linux/cdx/cdx_bus.h>
#include <linux/iommu.h>
#include <linux/dma-map-ops.h>
+#include <linux/debugfs.h>
#include "cdx.h"

/* Default DMA mask for devices on a CDX bus */
@@ -75,6 +76,8 @@
static DEFINE_IDA(cdx_controller_ida);
/* Lock to protect controller ops */
static DEFINE_MUTEX(cdx_controller_lock);
+/* Debugfs dir for cdx bus */
+static struct dentry *cdx_debugfs_dir;

static char *compat_node_name = "xlnx,versal-net-cdx";

@@ -149,6 +152,7 @@ static int cdx_unregister_device(struct device *dev,
cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
} else {
cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
+ debugfs_remove_recursive(cdx_dev->debugfs_dir);
kfree(cdx_dev->driver_override);
cdx_dev->driver_override = NULL;
}
@@ -552,6 +556,34 @@ static const struct attribute_group *cdx_dev_groups[] = {
NULL,
};

+static int cdx_debug_resource_show(struct seq_file *s, void *data)
+{
+ struct cdx_device *cdx_dev = s->private;
+ int i;
+
+ for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
+ struct resource *res = &cdx_dev->res[i];
+
+ seq_printf(s, "0x%016llx 0x%016llx 0x%016llx\n",
+ (unsigned long long)res->start,
+ (unsigned long long)res->end,
+ (unsigned long long)res->flags);
+ }
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(cdx_debug_resource);
+
+static void cdx_device_debugfs_init(struct cdx_device *cdx_dev)
+{
+ cdx_dev->debugfs_dir = debugfs_create_dir(dev_name(&cdx_dev->dev), cdx_debugfs_dir);
+ if (IS_ERR(cdx_dev->debugfs_dir))
+ return;
+
+ debugfs_create_file("resource", 0444, cdx_dev->debugfs_dir, cdx_dev,
+ &cdx_debug_resource_fops);
+}
+
static ssize_t rescan_store(const struct bus_type *bus,
const char *buf, size_t count)
{
@@ -809,6 +841,8 @@ int cdx_device_add(struct cdx_dev_params *dev_params)
}
}

+ cdx_device_debugfs_init(cdx_dev);
+
return 0;
resource_create_fail:
cdx_destroy_res_attr(cdx_dev, i);
@@ -913,6 +947,12 @@ EXPORT_SYMBOL_NS_GPL(cdx_unregister_controller, CDX_BUS_CONTROLLER);

static int __init cdx_bus_init(void)
{
- return bus_register(&cdx_bus_type);
+ int ret;
+
+ ret = bus_register(&cdx_bus_type);
+ if (!ret)
+ cdx_debugfs_dir = debugfs_create_dir(cdx_bus_type.name, NULL);
+
+ return ret;
}
postcore_initcall(cdx_bus_init);
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index 3096c31d5b9f..681fd4d644ab 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -128,6 +128,7 @@ struct cdx_controller {
* @dev_num: Device number for this device
* @res: array of MMIO region entries
* @res_attr: resource binary attribute
+ * @debugfs_dir: debugfs directory for this device
* @res_count: number of valid MMIO regions
* @dma_mask: Default DMA mask
* @flags: CDX device flags
@@ -155,6 +156,7 @@ struct cdx_device {
u8 dev_num;
struct resource res[MAX_CDX_DEV_RESOURCES];
struct bin_attribute *res_attr[MAX_CDX_DEV_RESOURCES];
+ struct dentry *debugfs_dir;
u8 res_count;
u64 dma_mask;
u16 flags;
--
2.34.1


2023-11-22 08:54:23

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] cdx: Create resource debugfs file for cdx device

On Wed, Nov 22, 2023 at 01:33:00PM +0530, Abhijit Gangurde wrote:
> resource debugfs file contains host addresses of CDX device resources.
> Each line of the resource file describe a region with start, end and
> flag fields.
>
> Signed-off-by: Abhijit Gangurde <[email protected]>
> ---
> drivers/cdx/cdx.c | 42 ++++++++++++++++++++++++++++++++++++-
> include/linux/cdx/cdx_bus.h | 2 ++
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> index 4edf64f9e98d..e01376272ed3 100644
> --- a/drivers/cdx/cdx.c
> +++ b/drivers/cdx/cdx.c
> @@ -65,6 +65,7 @@
> #include <linux/cdx/cdx_bus.h>
> #include <linux/iommu.h>
> #include <linux/dma-map-ops.h>
> +#include <linux/debugfs.h>
> #include "cdx.h"
>
> /* Default DMA mask for devices on a CDX bus */
> @@ -75,6 +76,8 @@
> static DEFINE_IDA(cdx_controller_ida);
> /* Lock to protect controller ops */
> static DEFINE_MUTEX(cdx_controller_lock);
> +/* Debugfs dir for cdx bus */
> +static struct dentry *cdx_debugfs_dir;
>
> static char *compat_node_name = "xlnx,versal-net-cdx";
>
> @@ -149,6 +152,7 @@ static int cdx_unregister_device(struct device *dev,
> cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
> } else {
> cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
> + debugfs_remove_recursive(cdx_dev->debugfs_dir);
> kfree(cdx_dev->driver_override);
> cdx_dev->driver_override = NULL;
> }
> @@ -552,6 +556,34 @@ static const struct attribute_group *cdx_dev_groups[] = {
> NULL,
> };
>
> +static int cdx_debug_resource_show(struct seq_file *s, void *data)
> +{
> + struct cdx_device *cdx_dev = s->private;
> + int i;
> +
> + for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
> + struct resource *res = &cdx_dev->res[i];
> +
> + seq_printf(s, "0x%016llx 0x%016llx 0x%016llx\n",
> + (unsigned long long)res->start,
> + (unsigned long long)res->end,
> + (unsigned long long)res->flags);

Why not just use %pR or %pr instead of creating your own style of
format?

thanks,

greg k-h

2023-11-22 16:19:30

by Gangurde, Abhijit

[permalink] [raw]
Subject: RE: [PATCH 2/2] cdx: Create resource debugfs file for cdx device

> > resource debugfs file contains host addresses of CDX device resources.
> > Each line of the resource file describe a region with start, end and
> > flag fields.
> >
> > Signed-off-by: Abhijit Gangurde <[email protected]>
> > ---
> > drivers/cdx/cdx.c | 42 ++++++++++++++++++++++++++++++++++++-
> > include/linux/cdx/cdx_bus.h | 2 ++
> > 2 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> > index 4edf64f9e98d..e01376272ed3 100644
> > --- a/drivers/cdx/cdx.c
> > +++ b/drivers/cdx/cdx.c
> > @@ -65,6 +65,7 @@
> > #include <linux/cdx/cdx_bus.h>
> > #include <linux/iommu.h>
> > #include <linux/dma-map-ops.h>
> > +#include <linux/debugfs.h>
> > #include "cdx.h"
> >
> > /* Default DMA mask for devices on a CDX bus */
> > @@ -75,6 +76,8 @@
> > static DEFINE_IDA(cdx_controller_ida);
> > /* Lock to protect controller ops */
> > static DEFINE_MUTEX(cdx_controller_lock);
> > +/* Debugfs dir for cdx bus */
> > +static struct dentry *cdx_debugfs_dir;
> >
> > static char *compat_node_name = "xlnx,versal-net-cdx";
> >
> > @@ -149,6 +152,7 @@ static int cdx_unregister_device(struct device *dev,
> > cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
> > } else {
> > cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
> > + debugfs_remove_recursive(cdx_dev->debugfs_dir);
> > kfree(cdx_dev->driver_override);
> > cdx_dev->driver_override = NULL;
> > }
> > @@ -552,6 +556,34 @@ static const struct attribute_group
> *cdx_dev_groups[] = {
> > NULL,
> > };
> >
> > +static int cdx_debug_resource_show(struct seq_file *s, void *data)
> > +{
> > + struct cdx_device *cdx_dev = s->private;
> > + int i;
> > +
> > + for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
> > + struct resource *res = &cdx_dev->res[i];
> > +
> > + seq_printf(s, "0x%016llx 0x%016llx 0x%016llx\n",
> > + (unsigned long long)res->start,
> > + (unsigned long long)res->end,
> > + (unsigned long long)res->flags);
>
> Why not just use %pR or %pr instead of creating your own style of
> format?

Will correct this in next spin.

Thanks,
Abhijit