On 3/2/20 5:52 PM, Andi Shyti wrote:
Hi Daniele,
I'm sorry I missed this patch,
On Thu, Feb 27, 2020 at 06:28:42PM -0800, Daniele Ceraolo Spurio wrote:
uC is a component of the GT, so it makes sense for the uC debugfs files
to be in the GT folder. A subfolder has been used to keep the same
structure we have for the code.
Can we please document the interface changes. I see there are
some differences between the original and the new interfaces.
What differences are you referring to? there aren't supposed to be any,
aside from the path change.
+#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name) \
+ static int __name ## _open(struct inode *inode, struct file *file) \
+{ \
+ return single_open(file, __name ## _show, inode->i_private); \
+} \
+static const struct file_operations __name ## _fops = { \
+ .owner = THIS_MODULE, \
+ .open = __name ## _open, \
+ .read = seq_read, \
+ .llseek = seq_lseek, \
+ .release = single_release, \
+}
Why do we need DEFINE_UC_DEBUGFS_ATTRIBUTE()?
DEFINE_GT_DEBUGFS_ATTRIBUTE() was meant to be common to all gt
debugfs. I there any reason we need a new one?
Just wanted to avoid including the other header just for this macro.
+struct debugfs_uc_file {
+ const char *name;
+ const struct file_operations *fops;
+};
+
+#define debugfs_uc_register_files(files__, root__, data__) \
+do { \
+ int i__ = 0; \
+ for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
+ debugfs_create_file(files__[i__].name, \
+ 0444, root__, data__, \
+ files__[i__].fops); \
+ } \
+} while (0)
You want to define your own debugfs_uc_register_files() instead
of using debugfs_gt_register_files() because you want "data__"
to be void, right?
I think we can achieve that by adding a wrapper in debugfs_gt.c,
perhaps we can do something like:
void __debugfs_gt_register_files(struct intel_gt *gt,
struct dentry *root,
const struct debugfs_gt_file *files,
void *data,
unsigned long count)
{
......
}
and
#define debugfs_gt_register_files(...) __debugfs_gt_register_files(...)
#define debugfs_uc_register_files(...) __debugfs_gt_register_files(...)
so that we can keep everything in a library. What do you think.
LGTM. Mind if I rename to:
intel_gt_debugfs_register(...)
intel_uc_debugfs_register(...)
to avoid the debugfs_* prefix, as pointed out by Jani?
Thanks,
Daniele
Thanks,
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
