[v1] drm/mxsfb/lcdif: use DRM_BRIDGE_ATTACH_NO_CONNECTOR and the bridge-connector

[PATCH 1/8] drm/mxsfb/lcdif: simplify remote pointer management using __free

Posted by Luca Ceresoli 2 weeks ago

Putting the remote device_node reference requires a of_node_put(ep) in both
error return points. Use a cleanup action to simplify the code.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/mxsfb/lcdif_drv.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 47da1d9336b9..756ca96373c8 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -53,16 +53,13 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
 	int ret;
 
 	for_each_endpoint_of_node(dev->of_node, ep) {
-		struct device_node *remote;
+		struct device_node *remote __free(drm_bridge_put) =
+			of_graph_get_remote_port_parent(ep);
 		struct of_endpoint of_ep;
 		struct drm_encoder *encoder;
 
-		remote = of_graph_get_remote_port_parent(ep);
-		if (!of_device_is_available(remote)) {
-			of_node_put(remote);
+		if (!of_device_is_available(remote))
 			continue;
-		}
-		of_node_put(remote);
 
 		ret = of_graph_parse_endpoint(ep, &of_ep);
 		if (ret < 0) {

-- 
2.53.0

Re: [PATCH 1/8] drm/mxsfb/lcdif: simplify remote pointer management using __free

Posted by Liu Ying 1 week, 2 days ago

Hi Luca,

On Fri, Mar 20, 2026 at 11:46:12AM +0100, Luca Ceresoli wrote:
> Putting the remote device_node reference requires a of_node_put(ep) in both

s/of_node_put(ep)/of_node_put(remote)/

> error return points.

Should be cleanup points instead?

> Use a cleanup action to simplify the code.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 47da1d9336b9..756ca96373c8 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -53,16 +53,13 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  	int ret;
>  
>  	for_each_endpoint_of_node(dev->of_node, ep) {
> -		struct device_node *remote;
> +		struct device_node *remote __free(drm_bridge_put) =

s/drm_bridge_put/device_node/, though I know you've already realized the
mistake in a separate reply.

With the above comments addressed:
Reviewed-by: Liu Ying <victor.liu@nxp.com>

> +			of_graph_get_remote_port_parent(ep);
>  		struct of_endpoint of_ep;
>  		struct drm_encoder *encoder;
>  
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!of_device_is_available(remote)) {
> -			of_node_put(remote);
> +		if (!of_device_is_available(remote))
>  			continue;
> -		}
> -		of_node_put(remote);
>  
>  		ret = of_graph_parse_endpoint(ep, &of_ep);
>  		if (ret < 0) {
> 

-- 
Regards,
Liu Ying

Re: [PATCH 1/8] drm/mxsfb/lcdif: simplify remote pointer management using __free

Posted by Liu Ying 1 week, 2 days ago

On Thu, Mar 26, 2026 at 02:40:25PM +0800, Liu Ying wrote:
> Hi Luca,
> 
> On Fri, Mar 20, 2026 at 11:46:12AM +0100, Luca Ceresoli wrote:
>> Putting the remote device_node reference requires a of_node_put(ep) in both
> 
> s/of_node_put(ep)/of_node_put(remote)/
> 
>> error return points.
> 
> Should be cleanup points instead?
> 
>> Use a cleanup action to simplify the code.
>>
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> ---
>>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> index 47da1d9336b9..756ca96373c8 100644
>> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
>> @@ -53,16 +53,13 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>>  	int ret;
>>  
>>  	for_each_endpoint_of_node(dev->of_node, ep) {
>> -		struct device_node *remote;
>> +		struct device_node *remote __free(drm_bridge_put) =
> 
> s/drm_bridge_put/device_node/, though I know you've already realized the
> mistake in a separate reply.
> 
> With the above comments addressed:

+1 comment: include linux/cleanup.h as __free() is used.

> Reviewed-by: Liu Ying <victor.liu@nxp.com>
> 
>> +			of_graph_get_remote_port_parent(ep);
>>  		struct of_endpoint of_ep;
>>  		struct drm_encoder *encoder;
>>  
>> -		remote = of_graph_get_remote_port_parent(ep);
>> -		if (!of_device_is_available(remote)) {
>> -			of_node_put(remote);
>> +		if (!of_device_is_available(remote))
>>  			continue;
>> -		}
>> -		of_node_put(remote);
>>  
>>  		ret = of_graph_parse_endpoint(ep, &of_ep);
>>  		if (ret < 0) {
>>
> 

-- 
Regards,
Liu Ying

Re: [PATCH 1/8] drm/mxsfb/lcdif: simplify remote pointer management using __free

Posted by Luca Ceresoli 2 weeks ago

Hello,

On Fri Mar 20, 2026 at 11:46 AM CET, Luca Ceresoli wrote:
> Putting the remote device_node reference requires a of_node_put(ep) in both
> error return points. Use a cleanup action to simplify the code.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/mxsfb/lcdif_drv.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index 47da1d9336b9..756ca96373c8 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -53,16 +53,13 @@ static int lcdif_attach_bridge(struct lcdif_drm_private *lcdif)
>  	int ret;
>
>  	for_each_endpoint_of_node(dev->of_node, ep) {
> -		struct device_node *remote;
> +		struct device_node *remote __free(drm_bridge_put) =
                                                  ^^^^^^^^^^^^^^

I just realized there's a mistake here, this should be
__free(device_node). However this does not prevent testing the series in
its entirety because patch 2 fixes this mistake.

Will be fixed in v2.

Sorry about the noise, did too many rebases today!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com