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@xxxxxxxxxx> 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@xxxxxxxxxxx> > > > > > > --- > > > > > > 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
