On Tue, Oct 08, 2019 at 05:02:04PM +0300, Heikki Krogerus wrote:
> On Tue, Oct 08, 2019 at 03:59:23PM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 08-10-2019 14:25, Heikki Krogerus wrote:
> > > Hi Hans,
> > >
> > > Fixed the compiler warning in this version. No other changes.
> > >
> > > The original cover letter:
> > >
> > > That AXP288 extcon driver is the last that uses build-in connection
> > > description. I'm replacing it with a code that finds the role mux
> > > software node instead.
> > >
> > > I'm proposing also here a little helper
> > > usb_role_switch_find_by_fwnode() that uses
> > > class_find_device_by_fwnode() to find the role switches.
> >
> > Both patches look good to me and I can confirm that things still
> > work as they should on a CHT device with an AXP288 PMIC, so for both:
> >
> > Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> >
> > Regards,
> >
> > Hans
> >
> > p.s.
> >
> > I guess this means we can remove the build-in connection (
> > device_connection_add / remove) stuff now?
>
> Yes. I'll prepare separate patches for that.
Actually, maybe it would make sense to just remove it in this series.
I'm attaching a patch that remove struct device_connection completely.
Can you check if it makes sense to you?
thanks,
--
heikki
>From 9444a9498dd22fd88208d19ab3454689b953eb7b Mon Sep 17 00:00:00 2001
From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
Date: Wed, 9 Oct 2019 16:49:24 +0300
Subject: [PATCH] device connection: Remove struct device_connection
Now that we have software fwnodes to cover cases where a
firmware node does not describe the connections, there is no
need for dedicated device connection descriptors.
This removes struct device_connection completely. From now
on the processing is always done with the fwnode handle.
Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
---
.../driver-api/device_connection.rst | 37 +-----
drivers/base/devcon.c | 116 ++++--------------
drivers/usb/roles/class.c | 12 +-
drivers/usb/typec/class.c | 12 +-
drivers/usb/typec/mux.c | 40 ++----
include/linux/device.h | 49 +-------
6 files changed, 48 insertions(+), 218 deletions(-)
diff --git a/Documentation/driver-api/device_connection.rst b/Documentation/driver-api/device_connection.rst
index ba364224c349..f1dfb426c0e8 100644
--- a/Documentation/driver-api/device_connection.rst
+++ b/Documentation/driver-api/device_connection.rst
@@ -5,39 +5,14 @@ Device connections
Introduction
------------
-Devices often have connections to other devices that are outside of the direct
-child/parent relationship. A serial or network communication controller, which
-could be a PCI device, may need to be able to get a reference to its PHY
-component, which could be attached for example to the I2C bus. Some device
-drivers need to be able to control the clocks or the GPIOs for their devices,
-and so on.
-
-Device connections are generic descriptions of any type of connection between
-two separate devices.
-
-Device connections alone do not create a dependency between the two devices.
-They are only descriptions which are not tied to either of the devices directly.
-A dependency between the two devices exists only if one of the two endpoint
-devices requests a reference to the other. The descriptions themselves can be
-defined in firmware (not yet supported) or they can be built-in.
-
-Usage
------
-
-Device connections should exist before device ``->probe`` callback is called for
-either endpoint device in the description. If the connections are defined in
-firmware, this is not a problem. It should be considered if the connection
-descriptions are "built-in", and need to be added separately.
-
-The connection description consists of the names of the two devices with the
-connection, i.e. the endpoints, and unique identifier for the connection which
-is needed if there are multiple connections between the two devices.
-
-After a description exists, the devices in it can request reference to the other
-endpoint device, or they can request the description itself.
+Device connection API allows drivers to acquire references to devices that are,
+according to their firmware nodes, connected to the devices those drivers are
+controlling. The API is in practice a wrapper that for now utilizes
+``fwnode_graph*()`` family of functions and
+:c:func:`fwnode_property_get_reference_arg` function.
API
---
.. kernel-doc:: drivers/base/devcon.c
- :functions: device_connection_find_match device_connection_find device_connection_add device_connection_remove
+ :functions: device_connection_find_match fwnode_connection_find_match device_connection_find
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 14e2178e09f8..f7698362f267 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -9,24 +9,20 @@
#include <linux/device.h>
#include <linux/property.h>
-static DEFINE_MUTEX(devcon_lock);
-static LIST_HEAD(devcon_list);
-
static void *
fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
void *data, devcon_match_fn_t match)
{
- struct device_connection con = { .id = con_id };
struct fwnode_handle *ep;
void *ret;
fwnode_graph_for_each_endpoint(fwnode, ep) {
- con.fwnode = fwnode_graph_get_remote_port_parent(ep);
- if (!fwnode_device_is_available(con.fwnode))
+ fwnode = fwnode_graph_get_remote_port_parent(ep);
+ if (!fwnode_device_is_available(fwnode))
continue;
- ret = match(&con, -1, data);
- fwnode_handle_put(con.fwnode);
+ ret = match(fwnode, con_id, data);
+ fwnode_handle_put(fwnode);
if (ret) {
fwnode_handle_put(ep);
return ret;
@@ -39,17 +35,16 @@ static void *
fwnode_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
void *data, devcon_match_fn_t match)
{
- struct device_connection con = { };
void *ret;
int i;
for (i = 0; ; i++) {
- con.fwnode = fwnode_find_reference(fwnode, con_id, i);
- if (IS_ERR(con.fwnode))
+ fwnode = fwnode_find_reference(fwnode, con_id, i);
+ if (IS_ERR(fwnode))
break;
- ret = match(&con, -1, data);
- fwnode_handle_put(con.fwnode);
+ ret = match(fwnode, NULL, data);
+ fwnode_handle_put(fwnode);
if (ret)
return ret;
}
@@ -99,37 +94,8 @@ EXPORT_SYMBOL_GPL(fwnode_connection_find_match);
void *device_connection_find_match(struct device *dev, const char *con_id,
void *data, devcon_match_fn_t match)
{
- struct fwnode_handle *fwnode = dev_fwnode(dev);
- const char *devname = dev_name(dev);
- struct device_connection *con;
- void *ret = NULL;
- int ep;
-
- if (!match)
- return NULL;
-
- ret = fwnode_connection_find_match(fwnode, con_id, data, match);
- if (ret)
- return ret;
-
- mutex_lock(&devcon_lock);
-
- list_for_each_entry(con, &devcon_list, list) {
- ep = match_string(con->endpoint, 2, devname);
- if (ep < 0)
- continue;
-
- if (con_id && strcmp(con->id, con_id))
- continue;
-
- ret = match(con, !ep, data);
- if (ret)
- break;
- }
-
- mutex_unlock(&devcon_lock);
-
- return ret;
+ return fwnode_connection_find_match(dev_fwnode(dev), con_id,
+ data, match);
}
EXPORT_SYMBOL_GPL(device_connection_find_match);
@@ -152,41 +118,25 @@ static struct bus_type *generic_match_buses[] = {
NULL,
};
-static void *device_connection_fwnode_match(struct device_connection *con)
+/* This tries to find the device from the most common bus types by name. */
+static void *generic_match(struct fwnode_handle *fwnode, const char *id,
+ void *data)
{
struct bus_type *bus;
struct device *dev;
+ void *ret = NULL;
for (bus = generic_match_buses[0]; bus; bus++) {
- dev = bus_find_device_by_fwnode(bus, con->fwnode);
- if (dev && !strncmp(dev_name(dev), con->id, strlen(con->id)))
- return dev;
+ dev = bus_find_device_by_fwnode(bus, fwnode);
+ if (dev) {
+ if (!strncmp(dev_name(dev), id, strlen(id)))
+ return dev;
+ ret = ERR_PTR(-EPROBE_DEFER);
+ }
put_device(dev);
}
- return NULL;
-}
-
-/* This tries to find the device from the most common bus types by name. */
-static void *generic_match(struct device_connection *con, int ep, void *data)
-{
- struct bus_type *bus;
- struct device *dev;
-
- if (con->fwnode)
- return device_connection_fwnode_match(con);
-
- for (bus = generic_match_buses[0]; bus; bus++) {
- dev = bus_find_device_by_name(bus, NULL, con->endpoint[ep]);
- if (dev)
- return dev;
- }
-
- /*
- * We only get called if a connection was found, tell the caller to
- * wait for the other device to show up.
- */
- return ERR_PTR(-EPROBE_DEFER);
+ return ret;
}
/**
@@ -205,27 +155,3 @@ struct device *device_connection_find(struct device *dev, const char *con_id)
return device_connection_find_match(dev, con_id, NULL, generic_match);
}
EXPORT_SYMBOL_GPL(device_connection_find);
-
-/**
- * device_connection_add - Register a connection description
- * @con: The connection description to be registered
- */
-void device_connection_add(struct device_connection *con)
-{
- mutex_lock(&devcon_lock);
- list_add_tail(&con->list, &devcon_list);
- mutex_unlock(&devcon_lock);
-}
-EXPORT_SYMBOL_GPL(device_connection_add);
-
-/**
- * device_connections_remove - Unregister connection description
- * @con: The connection description to be unregistered
- */
-void device_connection_remove(struct device_connection *con)
-{
- mutex_lock(&devcon_lock);
- list_del(&con->list);
- mutex_unlock(&devcon_lock);
-}
-EXPORT_SYMBOL_GPL(device_connection_remove);
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 8273126ffdf4..67e48c733e92 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -85,19 +85,15 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
}
EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
-static void *usb_role_switch_match(struct device_connection *con, int ep,
+static void *usb_role_switch_match(struct fwnode_handle *fwnode, const char *id,
void *data)
{
struct device *dev;
- if (con->fwnode) {
- if (con->id && !fwnode_property_present(con->fwnode, con->id))
- return NULL;
+ if (id && !fwnode_property_present(fwnode, id))
+ return NULL;
- dev = class_find_device_by_fwnode(role_class, con->fwnode);
- } else {
- dev = class_find_device_by_name(role_class, con->endpoint[ep]);
- }
+ dev = class_find_device_by_fwnode(role_class, fwnode);
return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 94a3eda62add..cccd60652618 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -205,20 +205,14 @@ static void typec_altmode_put_partner(struct altmode *altmode)
put_device(&adev->dev);
}
-static void *typec_port_match(struct device_connection *con, int ep, void *data)
+static void *typec_port_match(struct fwnode_handle *fwnode, const char *id,
+ void *data)
{
- struct device *dev;
-
/*
* FIXME: Check does the fwnode supports the requested SVID. If it does
* we need to return ERR_PTR(-PROBE_DEFER) when there is no device.
*/
- if (con->fwnode)
- return class_find_device_by_fwnode(typec_class, con->fwnode);
-
- dev = class_find_device_by_name(typec_class, con->endpoint[ep]);
-
- return dev ? dev : ERR_PTR(-EPROBE_DEFER);
+ return class_find_device_by_fwnode(typec_class, fwnode);
}
struct typec_altmode *
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 57907f26f681..957c405e4545 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -17,11 +17,6 @@
#include "bus.h"
-static int name_match(struct device *dev, const void *name)
-{
- return !strcmp((const char *)name, dev_name(dev));
-}
-
static bool dev_name_ends_with(struct device *dev, const char *suffix)
{
const char *name = dev_name(dev);
@@ -39,21 +34,16 @@ static int switch_fwnode_match(struct device *dev, const void *fwnode)
return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-switch");
}
-static void *typec_switch_match(struct device_connection *con, int ep,
+static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id,
void *data)
{
struct device *dev;
- if (con->fwnode) {
- if (con->id && !fwnode_property_present(con->fwnode, con->id))
- return NULL;
+ if (id && !fwnode_property_present(fwnode, id))
+ return NULL;
- dev = class_find_device(&typec_mux_class, NULL, con->fwnode,
- switch_fwnode_match);
- } else {
- dev = class_find_device(&typec_mux_class, NULL,
- con->endpoint[ep], name_match);
- }
+ dev = class_find_device(&typec_mux_class, NULL, fwnode,
+ switch_fwnode_match);
return dev ? to_typec_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}
@@ -182,7 +172,8 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode)
return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-mux");
}
-static void *typec_mux_match(struct device_connection *con, int ep, void *data)
+static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id,
+ void *data)
{
const struct typec_altmode_desc *desc = data;
struct device *dev;
@@ -191,31 +182,24 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
u16 *val;
int i;
- if (!con->fwnode) {
- dev = class_find_device(&typec_mux_class, NULL,
- con->endpoint[ep], name_match);
-
- return dev ? to_typec_switch(dev) : ERR_PTR(-EPROBE_DEFER);
- }
-
/*
* Check has the identifier already been "consumed". If it
* has, no need to do any extra connection identification.
*/
- match = !con->id;
+ match = !id;
if (match)
goto find_mux;
/* Accessory Mode muxes */
if (!desc) {
- match = fwnode_property_present(con->fwnode, "accessory");
+ match = fwnode_property_present(fwnode, "accessory");
if (match)
goto find_mux;
return NULL;
}
/* Alternate Mode muxes */
- nval = fwnode_property_count_u16(con->fwnode, "svid");
+ nval = fwnode_property_count_u16(fwnode, "svid");
if (nval <= 0)
return NULL;
@@ -223,7 +207,7 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
if (!val)
return ERR_PTR(-ENOMEM);
- nval = fwnode_property_read_u16_array(con->fwnode, "svid", val, nval);
+ nval = fwnode_property_read_u16_array(fwnode, "svid", val, nval);
if (nval < 0) {
kfree(val);
return ERR_PTR(nval);
@@ -240,7 +224,7 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
return NULL;
find_mux:
- dev = class_find_device(&typec_mux_class, NULL, con->fwnode,
+ dev = class_find_device(&typec_mux_class, NULL, fwnode,
mux_fwnode_match);
return dev ? to_typec_switch(dev) : ERR_PTR(-EPROBE_DEFER);
diff --git a/include/linux/device.h b/include/linux/device.h
index 9f2f2e169f95..26fcae1dae32 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1013,26 +1013,8 @@ struct device_dma_parameters {
unsigned long segment_boundary_mask;
};
-/**
- * struct device_connection - Device Connection Descriptor
- * @fwnode: The device node of the connected device
- * @endpoint: The names of the two devices connected together
- * @id: Unique identifier for the connection
- * @list: List head, private, for internal use only
- *
- * NOTE: @fwnode is not used together with @endpoint. @fwnode is used when
- * platform firmware defines the connection. When the connection is registered
- * with device_connection_add() @endpoint is used instead.
- */
-struct device_connection {
- struct fwnode_handle *fwnode;
- const char *endpoint[2];
- const char *id;
- struct list_head list;
-};
-
-typedef void *(*devcon_match_fn_t)(struct device_connection *con, int ep,
- void *data);
+typedef void *(*devcon_match_fn_t)(struct fwnode_handle *fwnode,
+ const char *id, void *data);
void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
const char *con_id, void *data,
@@ -1042,33 +1024,6 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
struct device *device_connection_find(struct device *dev, const char *con_id);
-void device_connection_add(struct device_connection *con);
-void device_connection_remove(struct device_connection *con);
-
-/**
- * device_connections_add - Add multiple device connections at once
- * @cons: Zero terminated array of device connection descriptors
- */
-static inline void device_connections_add(struct device_connection *cons)
-{
- struct device_connection *c;
-
- for (c = cons; c->endpoint[0]; c++)
- device_connection_add(c);
-}
-
-/**
- * device_connections_remove - Remove multiple device connections at once
- * @cons: Zero terminated array of device connection descriptors
- */
-static inline void device_connections_remove(struct device_connection *cons)
-{
- struct device_connection *c;
-
- for (c = cons; c->endpoint[0]; c++)
- device_connection_remove(c);
-}
-
/**
* enum device_link_state - Device link states.
* @DL_STATE_NONE: The presence of the drivers is not being tracked.
--
2.23.0
