diff mbox series

[v4,2/9] drm/bridge: simple-bridge: Use fwnode API to acquire device properties

Message ID 20240422191903.255642-3-sui.jingfeng@linux.dev (mailing list archive)
State New, archived
Headers show
Series drm/bridge: Allow using fwnode API to get the next bridge | expand

Commit Message

Sui Jingfeng April 22, 2024, 7:18 p.m. UTC
Make this driver less DT-dependent by calling the freshly created helpers,
should be no functional changes for DT based systems. But open the door for
otherwise use cases. Even though there is no user emerged yet, this still
do no harms.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/bridge/simple-bridge.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

Comments

Dmitry Baryshkov April 22, 2024, 7:56 p.m. UTC | #1
On Tue, Apr 23, 2024 at 03:18:56AM +0800, Sui Jingfeng wrote:
> Make this driver less DT-dependent by calling the freshly created helpers,
> should be no functional changes for DT based systems. But open the door for
> otherwise use cases. Even though there is no user emerged yet, this still
> do no harms.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/bridge/simple-bridge.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
> index 5813a2c4fc5e..3b09bdd5ad4d 100644
> --- a/drivers/gpu/drm/bridge/simple-bridge.c
> +++ b/drivers/gpu/drm/bridge/simple-bridge.c
> @@ -9,7 +9,6 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> -#include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
>  
> @@ -169,33 +168,32 @@ static const struct drm_bridge_funcs simple_bridge_bridge_funcs = {
>  
>  static int simple_bridge_probe(struct platform_device *pdev)
>  {
> +	struct fwnode_handle *fwnode = dev_fwnode(&pdev->dev);
>  	struct simple_bridge *sbridge;
> -	struct device_node *remote;
> +	int ret;
>  
>  	sbridge = devm_kzalloc(&pdev->dev, sizeof(*sbridge), GFP_KERNEL);
>  	if (!sbridge)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, sbridge);
>  
> -	sbridge->info = of_device_get_match_data(&pdev->dev);
> +	sbridge->info = device_get_match_data(&pdev->dev);
>  
>  	/* Get the next bridge in the pipeline. */
> -	remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
> -	if (!remote)
> -		return -EINVAL;
> -
> -	sbridge->next_bridge = of_drm_find_bridge(remote);
> -	of_node_put(remote);
> -
> +	sbridge->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);

Can we please stick to the interface of drm_of_find_panel_or_bridge()?

Also note, the driver isn't looking for the next_bridge. It is looking
for the bridge at the fwnode remote endpoint.

>  	if (!sbridge->next_bridge) {
>  		dev_dbg(&pdev->dev, "Next bridge not found, deferring probe\n");
>  		return -EPROBE_DEFER;
> +	} else if (IS_ERR(sbridge->next_bridge)) {
> +		ret = PTR_ERR(sbridge->next_bridge);
> +		dev_err(&pdev->dev, "Error on finding the next bridge: %d\n", ret);
> +		return ret;
>  	}
>  
>  	/* Get the regulator and GPIO resources. */
>  	sbridge->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>  	if (IS_ERR(sbridge->vdd)) {
> -		int ret = PTR_ERR(sbridge->vdd);
> +		ret = PTR_ERR(sbridge->vdd);
>  		if (ret == -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
>  		sbridge->vdd = NULL;
> @@ -210,9 +208,9 @@ static int simple_bridge_probe(struct platform_device *pdev)
>  
>  	/* Register the bridge. */
>  	sbridge->bridge.funcs = &simple_bridge_bridge_funcs;
> -	sbridge->bridge.of_node = pdev->dev.of_node;
>  	sbridge->bridge.timings = sbridge->info->timings;
>  
> +	drm_bridge_set_node(&sbridge->bridge, fwnode);

Please don't move the code. Having it in place of of_node setter
simplifies the review.

LGTM otherwise.

>  	drm_bridge_add(&sbridge->bridge);
>  
>  	return 0;
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
index 5813a2c4fc5e..3b09bdd5ad4d 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -9,7 +9,6 @@ 
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 
@@ -169,33 +168,32 @@  static const struct drm_bridge_funcs simple_bridge_bridge_funcs = {
 
 static int simple_bridge_probe(struct platform_device *pdev)
 {
+	struct fwnode_handle *fwnode = dev_fwnode(&pdev->dev);
 	struct simple_bridge *sbridge;
-	struct device_node *remote;
+	int ret;
 
 	sbridge = devm_kzalloc(&pdev->dev, sizeof(*sbridge), GFP_KERNEL);
 	if (!sbridge)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, sbridge);
 
-	sbridge->info = of_device_get_match_data(&pdev->dev);
+	sbridge->info = device_get_match_data(&pdev->dev);
 
 	/* Get the next bridge in the pipeline. */
-	remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
-	if (!remote)
-		return -EINVAL;
-
-	sbridge->next_bridge = of_drm_find_bridge(remote);
-	of_node_put(remote);
-
+	sbridge->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);
 	if (!sbridge->next_bridge) {
 		dev_dbg(&pdev->dev, "Next bridge not found, deferring probe\n");
 		return -EPROBE_DEFER;
+	} else if (IS_ERR(sbridge->next_bridge)) {
+		ret = PTR_ERR(sbridge->next_bridge);
+		dev_err(&pdev->dev, "Error on finding the next bridge: %d\n", ret);
+		return ret;
 	}
 
 	/* Get the regulator and GPIO resources. */
 	sbridge->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
 	if (IS_ERR(sbridge->vdd)) {
-		int ret = PTR_ERR(sbridge->vdd);
+		ret = PTR_ERR(sbridge->vdd);
 		if (ret == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 		sbridge->vdd = NULL;
@@ -210,9 +208,9 @@  static int simple_bridge_probe(struct platform_device *pdev)
 
 	/* Register the bridge. */
 	sbridge->bridge.funcs = &simple_bridge_bridge_funcs;
-	sbridge->bridge.of_node = pdev->dev.of_node;
 	sbridge->bridge.timings = sbridge->info->timings;
 
+	drm_bridge_set_node(&sbridge->bridge, fwnode);
 	drm_bridge_add(&sbridge->bridge);
 
 	return 0;