On Tue, 06 Dec 2022 01:20:59 -0800, Alan Previn wrote:
>
> GuC log relay debugfs name for the control handle vs the actual relay
> channel are vague. Fix them so it's obvious from the name.
No real objection to anything here, just a couple of questions.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 2 +-
> .../drm/i915/gt/uc/intel_guc_log_debugfs.c | 22 +++++++++----------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index 6e880d9f42d4..d019c60d34e8 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -551,7 +551,7 @@ static int guc_log_relay_create(struct intel_guc_log *log)
> */
> n_subbufs = intel_guc_log_relay_subbuf_count(log);
>
> - guc_log_relay_chan = relay_open("guc_log",
> + guc_log_relay_chan = relay_open("guc_log_relay_chan",
Is this a user visible name or just something internal? I.e. Is this a user
visible file name?
> dev_priv->drm.primary->debugfs_root,
> subbuf_size, n_subbufs,
> &relay_callbacks, dev_priv);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c
> index 27756640338e..feff1e606b38 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log_debugfs.c
> @@ -137,7 +137,7 @@ DEFINE_SIMPLE_ATTRIBUTE(guc_log_relay_subbuf_count_fops,
> guc_log_relay_subbuf_count_get, NULL,
> "%lld\n");
>
> -static int guc_log_relay_open(struct inode *inode, struct file *file)
> +static int guc_log_relay_ctl_open(struct inode *inode, struct file *file)
Again not objecting, but what is the purpose/thinking behind adding _ctl_
to these function names? The previous names seemed fine?
> {
> struct intel_guc_log *log = inode->i_private;
>
> @@ -150,10 +150,10 @@ static int guc_log_relay_open(struct inode *inode, struct file *file)
> }
>
> static ssize_t
> -guc_log_relay_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> +guc_log_relay_ctl_write(struct file *filp,
> + const char __user *ubuf,
> + size_t cnt,
> + loff_t *ppos)
> {
> struct intel_guc_log *log = filp->private_data;
> int val;
> @@ -175,7 +175,7 @@ guc_log_relay_write(struct file *filp,
> return ret ?: cnt;
> }
>
> -static int guc_log_relay_release(struct inode *inode, struct file *file)
> +static int guc_log_relay_ctl_release(struct inode *inode, struct file *file)
> {
> struct intel_guc_log *log = inode->i_private;
>
> @@ -183,11 +183,11 @@ static int guc_log_relay_release(struct inode *inode, struct file *file)
> return 0;
> }
>
> -static const struct file_operations guc_log_relay_fops = {
> +static const struct file_operations guc_log_relay_ctl_fops = {
> .owner = THIS_MODULE,
> - .open = guc_log_relay_open,
> - .write = guc_log_relay_write,
> - .release = guc_log_relay_release,
> + .open = guc_log_relay_ctl_open,
> + .write = guc_log_relay_ctl_write,
> + .release = guc_log_relay_ctl_release,
> };
>
> void intel_guc_log_debugfs_register(struct intel_guc_log *log,
> @@ -197,7 +197,7 @@ void intel_guc_log_debugfs_register(struct intel_guc_log *log,
> { "guc_log_dump", &guc_log_dump_fops, NULL },
> { "guc_load_err_log_dump", &guc_load_err_log_dump_fops, NULL },
> { "guc_log_level", &guc_log_level_fops, NULL },
> - { "guc_log_relay", &guc_log_relay_fops, NULL },
> + { "guc_log_relay_ctl", &guc_log_relay_ctl_fops, NULL },
> { "guc_log_relay_subbuf_size", &guc_log_relay_subbuf_size_fops, NULL },
> { "guc_log_relay_subbuf_count", &guc_log_relay_subbuf_count_fops, NULL },
> };
Thanks.
--
Ashutosh
