devm_drm_of_get_bridge() and drmm_of_get_bridge() do not have anything to
do with struct drm_panel anymore, they just manage bridges. So move them
from bridge/panel.c to drm_bridge.c.
Move also of_drm_find_bridge_by_endpoint() which is used only by
devm_drm_of_get_bridge() and drmm_of_get_bridge().
No code changes, only move functions to a different file within the same
module and add an #include as needed.
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
This patch was added in v6.
---
drivers/gpu/drm/bridge/panel.c | 102 -----------------------------------------
drivers/gpu/drm/drm_bridge.c | 100 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 100 insertions(+), 102 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 6995de605e7317dd1eb153afd475746ced764712..1230ae50b2020e7a9306cac83009dd600dd61d26 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -418,49 +418,6 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
}
EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
-/**
- * of_drm_find_bridge_by_endpoint - return drm_bridge connected to an endpoint
- * @np: device tree node containing encoder output ports
- * @port: port in the device tree node
- * @endpoint: endpoint in the device tree node
- * @bridge: pointer to hold returned drm_bridge (must not be NULL)
- *
- * Given a DT node's port and endpoint number, find the connected node and
- * return the associated struct drm_bridge.
- *
- * Returns zero if successful, or one of the standard error codes if it fails.
- */
-static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
- int port, int endpoint,
- struct drm_bridge **bridge)
-{
- int ret = -EPROBE_DEFER;
- struct device_node *remote;
-
- if (!bridge)
- return -EINVAL;
-
- /*
- * of_graph_get_remote_node() produces a noisy error message if port
- * node isn't found and the absence of the port is a legit case here,
- * so at first we silently check whether graph presents in the
- * device-tree node.
- */
- if (!of_graph_is_present(np))
- return -ENODEV;
-
- remote = of_graph_get_remote_node(np, port, endpoint);
- if (!remote)
- return -ENODEV;
-
- *bridge = of_drm_find_bridge(remote);
- if (*bridge)
- ret = 0;
-
- of_node_put(remote);
- return ret;
-}
-
/**
* of_drm_get_panel_orientation - look up the orientation of the panel through
* the "rotation" binding from a device tree node
@@ -1150,62 +1107,3 @@ struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge)
return &panel_bridge->connector;
}
EXPORT_SYMBOL(drm_panel_bridge_connector);
-
-#ifdef CONFIG_OF
-/**
- * devm_drm_of_get_bridge - Return next bridge in the chain
- * @dev: device to tie the bridge lifetime to
- * @np: device tree node containing encoder output ports
- * @port: port in the device tree node
- * @endpoint: endpoint in the device tree node
- *
- * Given a DT node's port and endpoint number, finds the connected node
- * and returns the associated bridge if any.
- *
- * Returns a pointer to the bridge if successful, or an error pointer
- * otherwise.
- */
-struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
- struct device_node *np,
- u32 port, u32 endpoint)
-{
- struct drm_bridge *bridge;
- int ret;
-
- ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
- if (ret)
- return ERR_PTR(ret);
-
- return bridge;
-}
-EXPORT_SYMBOL(devm_drm_of_get_bridge);
-
-/**
- * drmm_of_get_bridge - Return next bridge in the chain
- * @drm: device to tie the bridge lifetime to
- * @np: device tree node containing encoder output ports
- * @port: port in the device tree node
- * @endpoint: endpoint in the device tree node
- *
- * Given a DT node's port and endpoint number, finds the connected node
- * and returns the associated bridge if any.
- *
- * Returns a drmm managed pointer to the bridge if successful, or an error
- * pointer otherwise.
- */
-struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
- struct device_node *np,
- u32 port, u32 endpoint)
-{
- struct drm_bridge *bridge;
- int ret;
-
- ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
- if (ret)
- return ERR_PTR(ret);
-
- return bridge;
-}
-EXPORT_SYMBOL(drmm_of_get_bridge);
-
-#endif
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 87cebec2de806781cee22da54d666eee9bde3648..2aa17fbe538b86066c4e68f0d0e8046e9ca9b965 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -25,6 +25,7 @@
#include <linux/media-bus-format.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/of.h>
#include <drm/drm_atomic_state_helper.h>
#include <drm/drm_bridge.h>
@@ -1334,6 +1335,105 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
+
+/**
+ * of_drm_find_bridge_by_endpoint - return drm_bridge connected to an endpoint
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ * @bridge: pointer to hold returned drm_bridge (must not be NULL)
+ *
+ * Given a DT node's port and endpoint number, find the connected node and
+ * return the associated struct drm_bridge.
+ *
+ * Returns zero if successful, or one of the standard error codes if it fails.
+ */
+static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
+ int port, int endpoint,
+ struct drm_bridge **bridge)
+{
+ int ret = -EPROBE_DEFER;
+ struct device_node *remote;
+
+ if (!bridge)
+ return -EINVAL;
+
+ /*
+ * of_graph_get_remote_node() produces a noisy error message if port
+ * node isn't found and the absence of the port is a legit case here,
+ * so at first we silently check whether graph presents in the
+ * device-tree node.
+ */
+ if (!of_graph_is_present(np))
+ return -ENODEV;
+
+ remote = of_graph_get_remote_node(np, port, endpoint);
+ if (!remote)
+ return -ENODEV;
+
+ *bridge = of_drm_find_bridge(remote);
+ if (*bridge)
+ ret = 0;
+
+ of_node_put(remote);
+ return ret;
+}
+
+/**
+ * devm_drm_of_get_bridge - Return next bridge in the chain
+ * @dev: device to tie the bridge lifetime to
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ *
+ * Given a DT node's port and endpoint number, finds the connected node
+ * and returns the associated bridge if any.
+ *
+ * Returns a pointer to the bridge if successful, or an error pointer
+ * otherwise.
+ */
+struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
+ struct device_node *np,
+ u32 port, u32 endpoint)
+{
+ struct drm_bridge *bridge;
+ int ret;
+
+ ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return bridge;
+}
+EXPORT_SYMBOL(devm_drm_of_get_bridge);
+
+/**
+ * drmm_of_get_bridge - Return next bridge in the chain
+ * @drm: device to tie the bridge lifetime to
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ *
+ * Given a DT node's port and endpoint number, finds the connected node
+ * and returns the associated bridge if any.
+ *
+ * Returns a drmm managed pointer to the bridge if successful, or an error
+ * pointer otherwise.
+ */
+struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
+ struct device_node *np,
+ u32 port, u32 endpoint)
+{
+ struct drm_bridge *bridge;
+ int ret;
+
+ ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return bridge;
+}
+EXPORT_SYMBOL(drmm_of_get_bridge);
#endif
MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
--
2.34.1On Thu, Feb 06, 2025 at 07:14:24PM +0100, Luca Ceresoli wrote:
> devm_drm_of_get_bridge() and drmm_of_get_bridge() do not have anything to
> do with struct drm_panel anymore, they just manage bridges. So move them
> from bridge/panel.c to drm_bridge.c.
>
> Move also of_drm_find_bridge_by_endpoint() which is used only by
> devm_drm_of_get_bridge() and drmm_of_get_bridge().
>
> No code changes, only move functions to a different file within the same
> module and add an #include as needed.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
> This patch was added in v6.
> ---
> drivers/gpu/drm/bridge/panel.c | 102 -----------------------------------------
> drivers/gpu/drm/drm_bridge.c | 100 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 100 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index 6995de605e7317dd1eb153afd475746ced764712..1230ae50b2020e7a9306cac83009dd600dd61d26 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -418,49 +418,6 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
> }
> EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
>
> -/**
> - * of_drm_find_bridge_by_endpoint - return drm_bridge connected to an endpoint
> - * @np: device tree node containing encoder output ports
> - * @port: port in the device tree node
> - * @endpoint: endpoint in the device tree node
> - * @bridge: pointer to hold returned drm_bridge (must not be NULL)
> - *
> - * Given a DT node's port and endpoint number, find the connected node and
> - * return the associated struct drm_bridge.
> - *
> - * Returns zero if successful, or one of the standard error codes if it fails.
> - */
> -static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
> - int port, int endpoint,
> - struct drm_bridge **bridge)
I'd say make this function the main API instead (and name it drm_of
rather than of_drm, this can happen in the previous patch).
> -{
> - int ret = -EPROBE_DEFER;
> - struct device_node *remote;
> -
> - if (!bridge)
> - return -EINVAL;
> -
> - /*
> - * of_graph_get_remote_node() produces a noisy error message if port
> - * node isn't found and the absence of the port is a legit case here,
> - * so at first we silently check whether graph presents in the
> - * device-tree node.
> - */
> - if (!of_graph_is_present(np))
> - return -ENODEV;
> -
> - remote = of_graph_get_remote_node(np, port, endpoint);
> - if (!remote)
> - return -ENODEV;
> -
> - *bridge = of_drm_find_bridge(remote);
> - if (*bridge)
> - ret = 0;
> -
> - of_node_put(remote);
> - return ret;
> -}
> -
> /**
> * of_drm_get_panel_orientation - look up the orientation of the panel through
> * the "rotation" binding from a device tree node
> @@ -1150,62 +1107,3 @@ struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge)
> return &panel_bridge->connector;
> }
> EXPORT_SYMBOL(drm_panel_bridge_connector);
> -
> -#ifdef CONFIG_OF
> -/**
> - * devm_drm_of_get_bridge - Return next bridge in the chain
> - * @dev: device to tie the bridge lifetime to
> - * @np: device tree node containing encoder output ports
> - * @port: port in the device tree node
> - * @endpoint: endpoint in the device tree node
> - *
> - * Given a DT node's port and endpoint number, finds the connected node
> - * and returns the associated bridge if any.
> - *
> - * Returns a pointer to the bridge if successful, or an error pointer
> - * otherwise.
> - */
> -struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
> - struct device_node *np,
> - u32 port, u32 endpoint)
> -{
> - struct drm_bridge *bridge;
> - int ret;
> -
> - ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
> - if (ret)
> - return ERR_PTR(ret);
> -
> - return bridge;
> -}
Then these two functions can go into the header as static inlines with
the proper deprecation notification. There is little point in having
them in the source file. I think eventually all users should be
converted into using your new of_drm_find_bridge_by_endpoint() funcion.
> -EXPORT_SYMBOL(devm_drm_of_get_bridge);
> -
> -/**
> - * drmm_of_get_bridge - Return next bridge in the chain
> - * @drm: device to tie the bridge lifetime to
> - * @np: device tree node containing encoder output ports
> - * @port: port in the device tree node
> - * @endpoint: endpoint in the device tree node
> - *
> - * Given a DT node's port and endpoint number, finds the connected node
> - * and returns the associated bridge if any.
> - *
> - * Returns a drmm managed pointer to the bridge if successful, or an error
> - * pointer otherwise.
> - */
> -struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
> - struct device_node *np,
> - u32 port, u32 endpoint)
> -{
> - struct drm_bridge *bridge;
> - int ret;
> -
> - ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
> - if (ret)
> - return ERR_PTR(ret);
> -
> - return bridge;
> -}
> -EXPORT_SYMBOL(drmm_of_get_bridge);
> -
> -#endif
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 87cebec2de806781cee22da54d666eee9bde3648..2aa17fbe538b86066c4e68f0d0e8046e9ca9b965 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -25,6 +25,7 @@
> #include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/of.h>
>
> #include <drm/drm_atomic_state_helper.h>
> #include <drm/drm_bridge.h>
> @@ -1334,6 +1335,105 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
> return NULL;
> }
> EXPORT_SYMBOL(of_drm_find_bridge);
> +
> +/**
> + * of_drm_find_bridge_by_endpoint - return drm_bridge connected to an endpoint
> + * @np: device tree node containing encoder output ports
> + * @port: port in the device tree node
> + * @endpoint: endpoint in the device tree node
> + * @bridge: pointer to hold returned drm_bridge (must not be NULL)
> + *
> + * Given a DT node's port and endpoint number, find the connected node and
> + * return the associated struct drm_bridge.
> + *
> + * Returns zero if successful, or one of the standard error codes if it fails.
> + */
> +static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
> + int port, int endpoint,
> + struct drm_bridge **bridge)
> +{
> + int ret = -EPROBE_DEFER;
> + struct device_node *remote;
> +
> + if (!bridge)
> + return -EINVAL;
> +
> + /*
> + * of_graph_get_remote_node() produces a noisy error message if port
> + * node isn't found and the absence of the port is a legit case here,
> + * so at first we silently check whether graph presents in the
> + * device-tree node.
> + */
> + if (!of_graph_is_present(np))
> + return -ENODEV;
> +
> + remote = of_graph_get_remote_node(np, port, endpoint);
> + if (!remote)
> + return -ENODEV;
> +
> + *bridge = of_drm_find_bridge(remote);
> + if (*bridge)
> + ret = 0;
> +
> + of_node_put(remote);
> + return ret;
> +}
> +
> +/**
> + * devm_drm_of_get_bridge - Return next bridge in the chain
> + * @dev: device to tie the bridge lifetime to
> + * @np: device tree node containing encoder output ports
> + * @port: port in the device tree node
> + * @endpoint: endpoint in the device tree node
> + *
> + * Given a DT node's port and endpoint number, finds the connected node
> + * and returns the associated bridge if any.
> + *
> + * Returns a pointer to the bridge if successful, or an error pointer
> + * otherwise.
> + */
> +struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
> + struct device_node *np,
> + u32 port, u32 endpoint)
> +{
> + struct drm_bridge *bridge;
> + int ret;
> +
> + ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return bridge;
> +}
> +EXPORT_SYMBOL(devm_drm_of_get_bridge);
> +
> +/**
> + * drmm_of_get_bridge - Return next bridge in the chain
> + * @drm: device to tie the bridge lifetime to
> + * @np: device tree node containing encoder output ports
> + * @port: port in the device tree node
> + * @endpoint: endpoint in the device tree node
> + *
> + * Given a DT node's port and endpoint number, finds the connected node
> + * and returns the associated bridge if any.
> + *
> + * Returns a drmm managed pointer to the bridge if successful, or an error
> + * pointer otherwise.
> + */
> +struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
> + struct device_node *np,
> + u32 port, u32 endpoint)
> +{
> + struct drm_bridge *bridge;
> + int ret;
> +
> + ret = of_drm_find_bridge_by_endpoint(np, port, endpoint, &bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return bridge;
> +}
> +EXPORT_SYMBOL(drmm_of_get_bridge);
> #endif
>
> MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On Fri, 7 Feb 2025 04:52:20 +0200 Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > On Thu, Feb 06, 2025 at 07:14:24PM +0100, Luca Ceresoli wrote: > > devm_drm_of_get_bridge() and drmm_of_get_bridge() do not have anything to > > do with struct drm_panel anymore, they just manage bridges. So move them > > from bridge/panel.c to drm_bridge.c. > > > > Move also of_drm_find_bridge_by_endpoint() which is used only by > > devm_drm_of_get_bridge() and drmm_of_get_bridge(). > > > > No code changes, only move functions to a different file within the same > > module and add an #include as needed. > > > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > > > --- > > > > This patch was added in v6. > > --- > > drivers/gpu/drm/bridge/panel.c | 102 ----------------------------------------- > > drivers/gpu/drm/drm_bridge.c | 100 ++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 100 insertions(+), 102 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > index 6995de605e7317dd1eb153afd475746ced764712..1230ae50b2020e7a9306cac83009dd600dd61d26 100644 > > --- a/drivers/gpu/drm/bridge/panel.c > > +++ b/drivers/gpu/drm/bridge/panel.c > > @@ -418,49 +418,6 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, > > } > > EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); > > > > -/** > > - * of_drm_find_bridge_by_endpoint - return drm_bridge connected to an endpoint > > - * @np: device tree node containing encoder output ports > > - * @port: port in the device tree node > > - * @endpoint: endpoint in the device tree node > > - * @bridge: pointer to hold returned drm_bridge (must not be NULL) > > - * > > - * Given a DT node's port and endpoint number, find the connected node and > > - * return the associated struct drm_bridge. > > - * > > - * Returns zero if successful, or one of the standard error codes if it fails. > > - */ > > -static int of_drm_find_bridge_by_endpoint(const struct device_node *np, > > - int port, int endpoint, > > - struct drm_bridge **bridge) > > I'd say make this function the main API instead (and name it drm_of > rather than of_drm, this can happen in the previous patch). I agree there should be a small number of APIs for the foreseeable future (and any number of, hopefully decreasing-at-some-point, deprecated ones). And I agree this one ^ and the devm_drm_of_get_bridge() below are equivalent, despite having different signatures, and so one should disappear. So, time to think about what APIs we want. Some thoughts of mine: * I prefer "get" over "find", looks more intuitive as these functions will drm_bridge_get() * Is there a logic between of_drm_ and drm_of_? Just "the former is old and deprecated"? * Since getting bridges via the endpoint is the preferred way, I'd like this function to have a shorter name than its variants * Returning a struct drm_bridge err_ptr looks better to me than returning an error and the bridge via a ptr-to-ptr, especially as we don't have anymore the case of returning a panel or a bridge from the same function So, bottom line, we'd have: - struct drm_bridge *drm_of_get_bridge(np, port, endpoint) - struct drm_bridge *drm_of_get_bridge_by_node(bridge_np) - devm_ and drmm_ variants of the above or a subset of these, in case some is not needed. What are your opinions? > > -struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, > > - struct device_node *np, > > - u32 port, u32 endpoint) ^ kept for reference Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Fri, Feb 07, 2025 at 09:54:21AM +0100, Luca Ceresoli wrote: > On Fri, 7 Feb 2025 04:52:20 +0200 > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > > On Thu, Feb 06, 2025 at 07:14:24PM +0100, Luca Ceresoli wrote: > > > devm_drm_of_get_bridge() and drmm_of_get_bridge() do not have anything to > > > do with struct drm_panel anymore, they just manage bridges. So move them > > > from bridge/panel.c to drm_bridge.c. > > > > > > Move also of_drm_find_bridge_by_endpoint() which is used only by > > > devm_drm_of_get_bridge() and drmm_of_get_bridge(). > > > > > > No code changes, only move functions to a different file within the same > > > module and add an #include as needed. > > > > > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > > > > > --- > > > > > > This patch was added in v6. > > > --- > > > drivers/gpu/drm/bridge/panel.c | 102 ----------------------------------------- > > > drivers/gpu/drm/drm_bridge.c | 100 ++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 100 insertions(+), 102 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > > index 6995de605e7317dd1eb153afd475746ced764712..1230ae50b2020e7a9306cac83009dd600dd61d26 100644 > > > --- a/drivers/gpu/drm/bridge/panel.c > > > +++ b/drivers/gpu/drm/bridge/panel.c > > > @@ -418,49 +418,6 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, > > > } > > > EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge); > > > > > > -/** > > > - * of_drm_find_bridge_by_endpoint - return drm_bridge connected to an endpoint > > > - * @np: device tree node containing encoder output ports > > > - * @port: port in the device tree node > > > - * @endpoint: endpoint in the device tree node > > > - * @bridge: pointer to hold returned drm_bridge (must not be NULL) > > > - * > > > - * Given a DT node's port and endpoint number, find the connected node and > > > - * return the associated struct drm_bridge. > > > - * > > > - * Returns zero if successful, or one of the standard error codes if it fails. > > > - */ > > > -static int of_drm_find_bridge_by_endpoint(const struct device_node *np, > > > - int port, int endpoint, > > > - struct drm_bridge **bridge) > > > > I'd say make this function the main API instead (and name it drm_of > > rather than of_drm, this can happen in the previous patch). > > I agree there should be a small number of APIs for the foreseeable > future (and any number of, hopefully decreasing-at-some-point, > deprecated ones). > > And I agree this one ^ and the devm_drm_of_get_bridge() below are > equivalent, despite having different signatures, and so one should > disappear. > > So, time to think about what APIs we want. Some thoughts of mine: > > * I prefer "get" over "find", looks more intuitive as these functions > will drm_bridge_get() > * Is there a logic between of_drm_ and drm_of_? Just "the former is > old and deprecated"? I don't know, it might be historical. Nevertheless, I think, having just drm_ prefix for all DRM-related symbols is a good idea. > * Since getting bridges via the endpoint is the preferred way, I'd > like this function to have a shorter name than its variants > * Returning a struct drm_bridge err_ptr looks better to me than > returning an error and the bridge via a ptr-to-ptr, especially as we > don't have anymore the case of returning a panel or a bridge from > the same function > > So, bottom line, we'd have: > > - struct drm_bridge *drm_of_get_bridge(np, port, endpoint) > - struct drm_bridge *drm_of_get_bridge_by_node(bridge_np) I think these two are fine, please go with them > - devm_ and drmm_ variants of the above These two are only necessary for the refcounted bridges. I'd say, skip them as a part of the panel / bridge patchset. Please don't overcomplicate it too much. > > or a subset of these, in case some is not needed. > > What are your opinions? > > > > -struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, > > > - struct device_node *np, > > > - u32 port, u32 endpoint) > > ^ kept for reference > > Luca > > -- > Luca Ceresoli, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com -- With best wishes Dmitry
© 2016 - 2025 Red Hat, Inc.