Re: [PATCH v3 4/4] device property: Move fwnode graph ops to firmware specific locations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Cc Frank. (Oops.)

Sakari Ailus wrote:
Hi Rob,

Rob Herring wrote:
On Thu, Mar 16, 2017 at 8:34 AM, Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
Move firmware specific implementations of the fwnode graph operations to
firmware specific locations.

Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
---
 drivers/acpi/property.c | 61 ++++++++++++++++++++++++++++++++
 drivers/base/property.c | 92
+++----------------------------------------------
 drivers/of/base.c       | 70 +++++++++++++++++++++++++++++++++++++
 include/linux/fwnode.h  | 16 +++++++++
 4 files changed, 152 insertions(+), 87 deletions(-)

As I've mentioned before, this is just propagating APIs I would like
to get rid of. See my series cleaning up DRM users[1].

Why would you want to get rid of the existing APIs? They do fit well for
the purpose they're currently used for on V4L2, at least. That could be
because the current API was implemented *for* V4L2. If you'd like to add
a new one, then at least I'd very much like to keep what exists at the
moment.

Please see e.g. isp_fwnodes_parse() in
drivers/media/platform/omap3isp/isp.c . That function is generic enough
to be moved to the V4L2 framework, with a driver callback to handle
driver specific matters.


diff --git a/drivers/of/base.c b/drivers/of/base.c
index e483dd1..a8686a8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2624,6 +2624,71 @@ static struct fwnode_handle
*of_fwnode_get_named_child_node(
        return NULL;
 }

+static struct fwnode_handle *of_fwnode_graph_get_next_endpoint(
+       struct fwnode_handle *fwnode, struct fwnode_handle *prev)
+{
+       struct device_node *node;
+
+       node = of_graph_get_next_endpoint(to_of_node(fwnode),
+                                         to_of_node(prev));
+       if (node)
+               return of_fwnode_handle(node);
+
+       return NULL;
+}

For example, most callers of of_graph_get_next_endpoint really just
want port=0, endpoint=0 and then they want the remote node. As the
assignment of port and endpoint numbers are fixed for a device,
drivers should be explicit about which port/endpoint they want. The
actual handle to either the port or endpoint nodes are mostly
irrelevant. We only really need the endpoint node handles when we have
properties in the endpoints.

I certainly don't mind adding a new API if there are use cases the
existing one does not serve well. But please don't remove the
enumeration API. It's simply way more practical.

If you have one or two ports and an endpoint in each, perhaps it's ok
just to try check if one more more of them are around. Some devices may
have e.g. six ports today and I don't think that number will go down in
the future, it's rather going to increase.

The caller can also find the endpoints without knowing the endpoint
numbers in advance. In a number of devices' DT binding documentation it
is not explicitly stated that endpoint IDs have a special meaning. I'm
not fully certain everything would keep on working fine if the drivers
that previously did not care about endpoint numbers would start e.g.
requiring zero from now on.

An analogous interface is the _ENUM IOCTLs in V4L2, with the difference
that it's towards the user space.



--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux