diff mbox series

[1/5] drm/bridge: Add drm_bridge_find_by_fwnode() helper

Message ID 20240122163220.110788-2-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 Jan. 22, 2024, 4:32 p.m. UTC
Because ACPI based systems only has the fwnode associated, the of_node
member of struct device is NULL. To order to move things forward, we add
drm_bridge_find_by_fwnode() to extend the support.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/drm_bridge.c | 33 +++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     |  4 ++++
 2 files changed, 37 insertions(+)

Comments

Laurent Pinchart Jan. 23, 2024, 1:17 a.m. UTC | #1
Hi Sui,

Thank you for the patch.

On Tue, Jan 23, 2024 at 12:32:16AM +0800, Sui Jingfeng wrote:
> Because ACPI based systems only has the fwnode associated, the of_node
> member of struct device is NULL. To order to move things forward, we add
> drm_bridge_find_by_fwnode() to extend the support.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>

Could we switch completely to fwnode, instead of maintaining the fwnode
and OF options side-by-side ?

> ---
>  drivers/gpu/drm/drm_bridge.c | 33 +++++++++++++++++++++++++++++++++
>  include/drm/drm_bridge.h     |  4 ++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index cee3188adf3d..ffd969adc2fb 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1347,6 +1347,39 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
>  EXPORT_SYMBOL(of_drm_find_bridge);
>  #endif
>  
> +/**
> + * drm_bridge_find_by_fwnode - Find the bridge corresponding to the associated fwnode
> + *
> + * @fwnode: fwnode for which to find the matching drm_bridge
> + *
> + * This function looks up a drm_bridge based on its associated fwnode.
> + *
> + * RETURNS:
> + * A reference to the drm_bridge control structure if found, NULL on failure.
> + */
> +struct drm_bridge *drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode)
> +{
> +	struct drm_bridge *ret = NULL;
> +	struct drm_bridge *bridge;
> +
> +	if (!fwnode)
> +		return NULL;
> +
> +	mutex_lock(&bridge_lock);
> +
> +	list_for_each_entry(bridge, &bridge_list, list) {
> +		if (bridge->fwnode == fwnode) {
> +			ret = bridge;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&bridge_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_bridge_find_by_fwnode);
> +
>  MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
>  MODULE_DESCRIPTION("DRM bridge infrastructure");
>  MODULE_LICENSE("GPL and additional rights");
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index e39da5807ba7..fe3d5f4bf37f 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -720,6 +720,8 @@ struct drm_bridge {
>  	struct list_head chain_node;
>  	/** @of_node: device node pointer to the bridge */
>  	struct device_node *of_node;
> +	/** @fwnode: associated fwnode supplied by platform firmware */
> +	struct fwnode_handle *fwnode;
>  	/** @list: to keep track of all added bridges */
>  	struct list_head list;
>  	/**
> @@ -796,6 +798,8 @@ static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
>  }
>  #endif
>  
> +struct drm_bridge *drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode);
> +
>  /**
>   * drm_bridge_get_next_bridge() - Get the next bridge in the chain
>   * @bridge: bridge object
Sui Jingfeng Jan. 23, 2024, 8:01 a.m. UTC | #2
Hi,


Thanks a lot for your review :-)


On 2024/1/23 09:17, Laurent Pinchart wrote:
> Hi Sui,
>
> Thank you for the patch.
>
> On Tue, Jan 23, 2024 at 12:32:16AM +0800, Sui Jingfeng wrote:
>> Because ACPI based systems only has the fwnode associated, the of_node
>> member of struct device is NULL. To order to move things forward, we add
>> drm_bridge_find_by_fwnode() to extend the support.
>>
>> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> Could we switch completely to fwnode, instead of maintaining the fwnode
> and OF options side-by-side ?


The side-by-side approach allow us to migrate smoothly,
the main consideration is that the OF approach has been
works very well, it is flexible and very successful in
the embedded world.

It seems that the fwnode API could NOT replace the OF
options completely. For example, the'of_device_id' and 'of_match_table' related things are always there, there
are large well-established helpers and subroutines and
already formed as a standard. Some part of it may suffer
from backward compatibility problems.

So I want to leave some space to other programmers.
Maybe there are other programmers who feel that using
OF alone is enough for a specific problem(domain).
Laurent Pinchart Jan. 23, 2024, 3:15 p.m. UTC | #3
Hi Sui,

On Tue, Jan 23, 2024 at 04:01:28PM +0800, Sui Jingfeng wrote:
> On 2024/1/23 09:17, Laurent Pinchart wrote:
> > On Tue, Jan 23, 2024 at 12:32:16AM +0800, Sui Jingfeng wrote:
> >> Because ACPI based systems only has the fwnode associated, the of_node
> >> member of struct device is NULL. To order to move things forward, we add
> >> drm_bridge_find_by_fwnode() to extend the support.
> >>
> >> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> >
> > Could we switch completely to fwnode, instead of maintaining the fwnode
> > and OF options side-by-side ?
> 
> The side-by-side approach allow us to migrate smoothly,

But it increases the maintenance burden for the duration of the
migration. I fear the migration would span years, with nobody really
taking active care of it, and the OF and non-OF API will have a risk to
diverge.

> the main consideration is that the OF approach has been
> works very well, it is flexible and very successful in
> the embedded world.

fwnode is a superset of OF, so I don't expect issues switching from OF
to fwnode. For the non-OF, non-fwnode users, that's possibly a different
question.

> It seems that the fwnode API could NOT replace the OF
> options completely. For example, the'of_device_id' and 'of_match_table' related things are always there, there

Yes, and that's not a problem. OF drivers still use of_device_id and
of_match_table, even if they use the fwnode API. No issue there.

> are large well-established helpers and subroutines and
> already formed as a standard. Some part of it may suffer
> from backward compatibility problems.

fwnode has been designed to offer the same API as OF for drivers. If
something is missing, it can be raised with the maintainers.

> So I want to leave some space to other programmers.
> Maybe there are other programmers who feel that using
> OF alone is enough for a specific problem(domain).
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index cee3188adf3d..ffd969adc2fb 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1347,6 +1347,39 @@  struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 EXPORT_SYMBOL(of_drm_find_bridge);
 #endif
 
+/**
+ * drm_bridge_find_by_fwnode - Find the bridge corresponding to the associated fwnode
+ *
+ * @fwnode: fwnode for which to find the matching drm_bridge
+ *
+ * This function looks up a drm_bridge based on its associated fwnode.
+ *
+ * RETURNS:
+ * A reference to the drm_bridge control structure if found, NULL on failure.
+ */
+struct drm_bridge *drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode)
+{
+	struct drm_bridge *ret = NULL;
+	struct drm_bridge *bridge;
+
+	if (!fwnode)
+		return NULL;
+
+	mutex_lock(&bridge_lock);
+
+	list_for_each_entry(bridge, &bridge_list, list) {
+		if (bridge->fwnode == fwnode) {
+			ret = bridge;
+			break;
+		}
+	}
+
+	mutex_unlock(&bridge_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_bridge_find_by_fwnode);
+
 MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
 MODULE_DESCRIPTION("DRM bridge infrastructure");
 MODULE_LICENSE("GPL and additional rights");
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index e39da5807ba7..fe3d5f4bf37f 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -720,6 +720,8 @@  struct drm_bridge {
 	struct list_head chain_node;
 	/** @of_node: device node pointer to the bridge */
 	struct device_node *of_node;
+	/** @fwnode: associated fwnode supplied by platform firmware */
+	struct fwnode_handle *fwnode;
 	/** @list: to keep track of all added bridges */
 	struct list_head list;
 	/**
@@ -796,6 +798,8 @@  static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 }
 #endif
 
+struct drm_bridge *drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode);
+
 /**
  * drm_bridge_get_next_bridge() - Get the next bridge in the chain
  * @bridge: bridge object