On Sat, Mar 15, 2025 at 09:17:16PM +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.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> Only build tested, but following the same pattern as the tested
> ti-sn65dsi86.
>
> Changes since v1:
> * switch from 'client->debugfs' to DRM 'debugfs_init' callback
>
> drivers/gpu/drm/bridge/ite-it6505.c | 59 ++++++++++-------------------
> 1 file changed, 19 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 88ef76a37fe6..803bfe9954e6 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -302,7 +302,6 @@
> #define WORD_LENGTH_18BIT 1
> #define WORD_LENGTH_20BIT 2
> #define WORD_LENGTH_24BIT 3
> -#define DEBUGFS_DIR_NAME "it6505-debugfs"
> #define READ_BUFFER_SIZE 400
>
> /* Vendor option */
> @@ -478,7 +477,6 @@ struct it6505 {
> struct device *codec_dev;
> struct delayed_work delayed_audio;
> struct it6505_audio_data audio;
> - struct dentry *debugfs;
>
> /* it6505 driver hold option */
> bool enable_drv_hold;
> @@ -3302,21 +3300,6 @@ static const struct drm_edid *it6505_bridge_edid_read(struct drm_bridge *bridge,
> return drm_edid_dup(it6505->cached_edid);
> }
>
> -static const struct drm_bridge_funcs it6505_bridge_funcs = {
> - .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> - .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> - .atomic_reset = drm_atomic_helper_bridge_reset,
> - .attach = it6505_bridge_attach,
> - .detach = it6505_bridge_detach,
> - .mode_valid = it6505_bridge_mode_valid,
> - .atomic_enable = it6505_bridge_atomic_enable,
> - .atomic_disable = it6505_bridge_atomic_disable,
> - .atomic_pre_enable = it6505_bridge_atomic_pre_enable,
> - .atomic_post_disable = it6505_bridge_atomic_post_disable,
> - .detect = it6505_bridge_detect,
> - .edid_read = it6505_bridge_edid_read,
> -};
> -
> static __maybe_unused int it6505_bridge_resume(struct device *dev)
> {
> struct it6505 *it6505 = dev_get_drvdata(dev);
> @@ -3574,36 +3557,34 @@ static const struct debugfs_entries debugfs_entry[] = {
> { NULL, NULL },
> };
>
> -static void debugfs_create_files(struct it6505 *it6505)
> +static void it6505_debugfs_init(struct drm_bridge *bridge, struct dentry *root)
> {
> + struct it6505 *it6505 = bridge_to_it6505(bridge);
> + struct dentry *debugfs = debugfs_create_dir(dev_name(it6505->dev), root);
> int i = 0;
>
> while (debugfs_entry[i].name && debugfs_entry[i].fops) {
> debugfs_create_file(debugfs_entry[i].name, 0644,
> - it6505->debugfs, it6505,
> - debugfs_entry[i].fops);
> + debugfs, it6505, debugfs_entry[i].fops);
> i++;
> }
> }
Ni: I'd say, it's better to move this function rather than moving
it6505_bridge_funcs. LGTM otherwise.
>
> -static void debugfs_init(struct it6505 *it6505)
> -{
> - struct device *dev = it6505->dev;
> -
> - it6505->debugfs = debugfs_create_dir(DEBUGFS_DIR_NAME, NULL);
> -
> - if (IS_ERR(it6505->debugfs)) {
> - dev_err(dev, "failed to create debugfs root");
> - return;
> - }
> -
> - debugfs_create_files(it6505);
> -}
> -
> -static void it6505_debugfs_remove(struct it6505 *it6505)
> -{
> - debugfs_remove_recursive(it6505->debugfs);
> -}
> +static const struct drm_bridge_funcs it6505_bridge_funcs = {
> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> + .atomic_reset = drm_atomic_helper_bridge_reset,
> + .attach = it6505_bridge_attach,
> + .detach = it6505_bridge_detach,
> + .mode_valid = it6505_bridge_mode_valid,
> + .atomic_enable = it6505_bridge_atomic_enable,
> + .atomic_disable = it6505_bridge_atomic_disable,
> + .atomic_pre_enable = it6505_bridge_atomic_pre_enable,
> + .atomic_post_disable = it6505_bridge_atomic_post_disable,
> + .detect = it6505_bridge_detect,
> + .edid_read = it6505_bridge_edid_read,
> + .debugfs_init = it6505_debugfs_init,
> +};
>
> static void it6505_shutdown(struct i2c_client *client)
> {
> @@ -3689,7 +3670,6 @@ static int it6505_i2c_probe(struct i2c_client *client)
> it6505_poweron(it6505);
>
> DRM_DEV_DEBUG_DRIVER(dev, "it6505 device name: %s", dev_name(dev));
> - debugfs_init(it6505);
> pm_runtime_enable(dev);
>
> it6505->aux.name = "DP-AUX";
> @@ -3712,7 +3692,6 @@ static void it6505_i2c_remove(struct i2c_client *client)
>
> drm_bridge_remove(&it6505->bridge);
> drm_dp_aux_unregister(&it6505->aux);
> - it6505_debugfs_remove(it6505);
> it6505_poweroff(it6505);
> it6505_remove_edid(it6505);
> }
> --
> 2.47.2
>
--
With best wishes
Dmitry
