[Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Rename GuC log relay debugfs descriptively

[Dixit, Ashutosh]

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 at intel.com>
> ---
>  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