[PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Wolfram Sang]

Do not create a custom directory in debugfs-root, but use the
debugfs_init callback to create a custom directory at the given place
for the bridge. The new directory layout looks like this on a Renesas
GrayHawk-Single with a R-Car V4M SoC:

	/sys/kernel/debug/dri/feb00000.display/DP-1/1-002c

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes since v1:
* switch from 'client->debugfs' to DRM 'debugfs_init' callback
* remove RFT because tested on hardware

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++--------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index e4d9006b59f1..87fffaa52bb0 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -424,36 +424,8 @@ static int status_show(struct seq_file *s, void *data)
 
 	return 0;
 }
-
 DEFINE_SHOW_ATTRIBUTE(status);
 
-static void ti_sn65dsi86_debugfs_remove(void *data)
-{
-	debugfs_remove_recursive(data);
-}
-
-static void ti_sn65dsi86_debugfs_init(struct ti_sn65dsi86 *pdata)
-{
-	struct device *dev = pdata->dev;
-	struct dentry *debugfs;
-	int ret;
-
-	debugfs = debugfs_create_dir(dev_name(dev), NULL);
-
-	/*
-	 * We might get an error back if debugfs wasn't enabled in the kernel
-	 * so let's just silently return upon failure.
-	 */
-	if (IS_ERR_OR_NULL(debugfs))
-		return;
-
-	ret = devm_add_action_or_reset(dev, ti_sn65dsi86_debugfs_remove, debugfs);
-	if (ret)
-		return;
-
-	debugfs_create_file("status", 0600, debugfs, pdata, &status_fops);
-}
-
 /* -----------------------------------------------------------------------------
  * Auxiliary Devices (*not* AUX)
  */
@@ -1215,6 +1187,15 @@ static const struct drm_edid *ti_sn_bridge_edid_read(struct drm_bridge *bridge,
 	return drm_edid_read_ddc(connector, &pdata->aux.ddc);
 }
 
+static void ti_sn65dsi86_debugfs_init(struct drm_bridge *bridge, struct dentry *root)
+{
+	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
+	struct dentry *debugfs;
+
+	debugfs = debugfs_create_dir(dev_name(pdata->dev), root);
+	debugfs_create_file("status", 0600, debugfs, pdata, &status_fops);
+}
+
 static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
 	.attach = ti_sn_bridge_attach,
 	.detach = ti_sn_bridge_detach,
@@ -1228,6 +1209,7 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.debugfs_init = ti_sn65dsi86_debugfs_init,
 };
 
 static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata,
@@ -1936,8 +1918,6 @@ static int ti_sn65dsi86_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ti_sn65dsi86_debugfs_init(pdata);
-
 	/*
 	 * Break ourselves up into a collection of aux devices. The only real
 	 * motiviation here is to solve the chicken-and-egg problem of probe
-- 
2.47.2

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Dmitry Baryshkov]

On Sat, Mar 15, 2025 at 09:15:11PM +0100, Wolfram Sang wrote:
> Do not create a custom directory in debugfs-root, but use the
> debugfs_init callback to create a custom directory at the given place
> for the bridge. The new directory layout looks like this on a Renesas
> GrayHawk-Single with a R-Car V4M SoC:
> 
> 	/sys/kernel/debug/dri/feb00000.display/DP-1/1-002c

Would you rather create a subdir using the bridge name (ti_sn65dsi86)
rather than dev_name? I don't know if we have an established practice.

> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Changes since v1:
> * switch from 'client->debugfs' to DRM 'debugfs_init' callback
> * remove RFT because tested on hardware
> 
-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On Sun, Mar 16, 2025 at 12:43:48AM +0200, Dmitry Baryshkov wrote:
> On Sat, Mar 15, 2025 at 09:15:11PM +0100, Wolfram Sang wrote:
> > Do not create a custom directory in debugfs-root, but use the
> > debugfs_init callback to create a custom directory at the given place
> > for the bridge. The new directory layout looks like this on a Renesas
> > GrayHawk-Single with a R-Car V4M SoC:
> > 
> > 	/sys/kernel/debug/dri/feb00000.display/DP-1/1-002c
> 
> Would you rather create a subdir using the bridge name (ti_sn65dsi86)
> rather than dev_name? I don't know if we have an established practice.

We don't have a practice yet. Keeping dev_name() was Doug's suggestion.
But I like your idea better, so I will change to it. Thank you for it!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]


> We don't have a practice yet. Keeping dev_name() was Doug's suggestion.
> But I like your idea better, so I will change to it. Thank you for it!

Just to make sure: there can only be one bridge, right? Because the
suggested name is not unique.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Laurent Pinchart]

On Sun, Mar 16, 2025 at 08:10:03AM +0100, Wolfram Sang wrote:
> 
> > We don't have a practice yet. Keeping dev_name() was Doug's suggestion.
> > But I like your idea better, so I will change to it. Thank you for it!
> 
> Just to make sure: there can only be one bridge, right? Because the
> suggested name is not unique.

Bridges can be chained. It's highly unlikely that a chain would contain
multiple bridges of the same model, as that would be quite pointless,
but in theory it could happen.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]


> > Just to make sure: there can only be one bridge, right? Because the
> > suggested name is not unique.
> 
> Bridges can be chained. It's highly unlikely that a chain would contain
> multiple bridges of the same model, as that would be quite pointless,
> but in theory it could happen.

Thanks for the input, Laurent. I suggest to keep the unique name then.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Dmitry Baryshkov]

On Sun, Mar 16, 2025 at 11:47:04AM +0100, Wolfram Sang wrote:
> 
> > > Just to make sure: there can only be one bridge, right? Because the
> > > suggested name is not unique.
> > 
> > Bridges can be chained. It's highly unlikely that a chain would contain
> > multiple bridges of the same model, as that would be quite pointless,
> > but in theory it could happen.
> 
> Thanks for the input, Laurent. I suggest to keep the unique name then.


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Doug Anderson]

Hi,

On Sat, Mar 15, 2025 at 1:17 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Do not create a custom directory in debugfs-root, but use the
> debugfs_init callback to create a custom directory at the given place
> for the bridge. The new directory layout looks like this on a Renesas
> GrayHawk-Single with a R-Car V4M SoC:
>
>         /sys/kernel/debug/dri/feb00000.display/DP-1/1-002c
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Changes since v1:
> * switch from 'client->debugfs' to DRM 'debugfs_init' callback
> * remove RFT because tested on hardware
>
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++--------------------
>  1 file changed, 10 insertions(+), 30 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'll plan to apply this next week assuming freedesktop is back up.

-Doug

Re: [PATCH v2] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback

[Doug Anderson]

Hi,

On Mon, Mar 17, 2025 at 7:33 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sat, Mar 15, 2025 at 1:17 PM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> >
> > Do not create a custom directory in debugfs-root, but use the
> > debugfs_init callback to create a custom directory at the given place
> > for the bridge. The new directory layout looks like this on a Renesas
> > GrayHawk-Single with a R-Car V4M SoC:
> >
> >         /sys/kernel/debug/dri/feb00000.display/DP-1/1-002c
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >
> > Changes since v1:
> > * switch from 'client->debugfs' to DRM 'debugfs_init' callback
> > * remove RFT because tested on hardware
> >
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 40 +++++++--------------------
> >  1 file changed, 10 insertions(+), 30 deletions(-)
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I'll plan to apply this next week assuming freedesktop is back up.

Pushed to drm-misc-next:

[1/1] drm/bridge: ti-sn65dsi86: make use of debugfs_init callback
      commit: 1d1f7b15cb9c11974cebfd39da51dc69b8cb31ff