On 3/5/20 10:02 AM, Andi Shyti wrote:
Hi Daniele,
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.
Have I seen it wrong or there are new files in this patch?
No, no new debugfs files, only the old ones moved across.
In any case, maybe we need to have the new structure.
+#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.
well that was supposed to be a library for all the gem/debugfs
files and avoid duplicated code, I don't see anything wrong with
including the file.
+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?
I have a patch for it, can you please hold a little, unless, of
course, yours is already ready.
Sure, I'll wait for your patch to land first.
Daniele
Obvously, the naming you propose makes sense.
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
