Hi,
I'm only presenting my idea with these how I think we should be able
to deal with graphs in the API, so these are completely untested, and
obviously I can't say for certain if the idea works or not.
I will try to test these using custom ACPI tables, but of course these
should be tested on DT platform as well, so if somebody can do that, I
would much appreciate.
Thanks,
Heikki Krogerus (5):
driver core: Add fwnode member to struct device_connection
usb: typec: mux: Find the muxes by also matching against the device
node
usb: roles: Find the muxes by also matching against the device node
usb: typec: Find the ports by also matching against the device node
drivers core: Find device connections also from device graphs
drivers/base/devcon.c | 48 +++++++++++++++++++++++++++++++++++---
drivers/usb/common/roles.c | 16 ++++++++++---
drivers/usb/typec/class.c | 19 ++++++++++++---
drivers/usb/typec/mux.c | 19 +++++++++++----
include/linux/device.h | 6 +++++
5 files changed, 95 insertions(+), 13 deletions(-)
--
2.19.1
If connections between devices are described in OF graph or
ACPI device graph, we can find them by using the
fwnode_graph_*() functions.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/base/devcon.c | 48 ++++++++++++++++++++++++++++++++++++++++---
1 file changed, 45 insertions(+), 3 deletions(-)
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index d427e806cd73..9c446ef84dd1 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -7,10 +7,45 @@
*/
#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,
+ void *(*match)(struct device_connection *con,
+ int ep, void *data))
+{
+ struct device_connection con = { .id = con_id };
+ struct fwnode_handle *ep;
+ const char *val;
+ void *ret;
+ int err;
+
+ fwnode_graph_for_each_endpoint(fwnode, ep) {
+ con.fwnode = fwnode_graph_get_remote_port_parent(ep);
+
+ if (con_id) {
+ err = fwnode_property_read_string(con.fwnode, "name",
+ &val);
+ if (err || strcmp(con_id, val)) {
+ fwnode_handle_put(con.fwnode);
+ continue;
+ }
+ }
+
+ ret = match(&con, -1, data);
+ fwnode_handle_put(con.fwnode);
+ if (ret) {
+ fwnode_handle_put(ep);
+ return ret;
+ }
+ }
+ return NULL;
+}
+
/**
* device_connection_find_match - Find physical connection to a device
* @dev: Device with the connection
@@ -23,10 +58,11 @@ static LIST_HEAD(devcon_list);
* caller is expecting to be returned.
*/
void *device_connection_find_match(struct device *dev, const char *con_id,
- void *data,
- void *(*match)(struct device_connection *con,
- int ep, void *data))
+ void *data,
+ void *(*match)(struct device_connection *con,
+ int ep, void *data))
{
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
const char *devname = dev_name(dev);
struct device_connection *con;
void *ret = NULL;
@@ -35,6 +71,12 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
if (!match)
return NULL;
+ if (fwnode) {
+ ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
+ if (ret)
+ return ret;
+ }
+
mutex_lock(&devcon_lock);
list_for_each_entry(con, &devcon_list, list) {
--
2.19.1
When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/typec/class.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 5db0593ca0bd..fe6f3a932a88 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -204,15 +204,28 @@ static void typec_altmode_put_partner(struct altmode *altmode)
put_device(&adev->dev);
}
-static int __typec_port_match(struct device *dev, const void *name)
+static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
+{
+ return dev_name(fwnode) == fwnode;
+}
+
+static int typec_port_name_match(struct device *dev, const void *name)
{
return !strcmp((const char *)name, dev_name(dev));
}
static void *typec_port_match(struct device_connection *con, int ep, void *data)
{
- return class_find_device(typec_class, NULL, con->endpoint[ep],
- __typec_port_match);
+ struct device *dev;
+
+ if (con->fwnode)
+ dev = class_find_device(typec_class, NULL, con->fwnode,
+ typec_port_fwnode_match);
+ else
+ dev = class_find_device(typec_class, NULL, con->endpoint[ep],
+ typec_port_name_match);
+
+ return dev ? dev : ERR_PTR(-EPROBE_DEFER);
}
struct typec_altmode *
--
2.19.1
When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/common/roles.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 99116af07f1d..bb52e006d203 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -8,6 +8,7 @@
*/
#include <linux/usb/role.h>
+#include <linux/property.h>
#include <linux/device.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
}
EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
-static int __switch_match(struct device *dev, const void *name)
+static int switch_fwnode_match(struct device *dev, const void *fwnode)
+{
+ return dev_fwnode(dev) == fwnode;
+}
+
+static int switch_name_match(struct device *dev, const void *name)
{
return !strcmp((const char *)name, dev_name(dev));
}
@@ -94,8 +100,12 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
{
struct device *dev;
- dev = class_find_device(role_class, NULL, con->endpoint[ep],
- __switch_match);
+ if (con->fwnode)
+ dev = class_find_device(role_class, NULL, con->fwnode,
+ switch_fwnode_match);
+ else
+ dev = class_find_device(role_class, NULL, con->endpoint[ep],
+ switch_name_match);
return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}
--
2.19.1
When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/typec/mux.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index d990aa510fab..161a96280296 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -11,6 +11,7 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/property.h>
#include <linux/usb/typec_mux.h>
static DEFINE_MUTEX(switch_lock);
@@ -23,9 +24,14 @@ static void *typec_switch_match(struct device_connection *con, int ep,
{
struct typec_switch *sw;
- list_for_each_entry(sw, &switch_list, entry)
- if (!strcmp(con->endpoint[ep], dev_name(sw->dev)))
+ list_for_each_entry(sw, &switch_list, entry) {
+ if (con->fwnode) {
+ if (dev_fwnode(sw->dev) == con->fwnode)
+ return sw;
+ } else if (!strcmp(con->endpoint[ep], dev_name(sw->dev))) {
return sw;
+ }
+ }
/*
* We only get called if a connection was found, tell the caller to
@@ -114,9 +120,14 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
{
struct typec_mux *mux;
- list_for_each_entry(mux, &mux_list, entry)
- if (!strcmp(con->endpoint[ep], dev_name(mux->dev)))
+ list_for_each_entry(mux, &mux_list, entry) {
+ if (con->fwnode) {
+ if (dev_fwnode(mux->dev) == con->fwnode)
+ return mux;
+ } else if (!strcmp(con->endpoint[ep], dev_name(mux->dev))) {
return mux;
+ }
+ }
/*
* We only get called if a connection was found, tell the caller to
--
2.19.1
This will prepare the device connection API for connections
described in firmware.
Signed-off-by: Heikki Krogerus <[email protected]>
---
include/linux/device.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 90224e75ade4..a964a0d614fa 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -753,11 +753,17 @@ struct device_dma_parameters {
/**
* 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 registeded
+ * 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;
--
2.19.1
On 10/24/18 8:05 AM, Heikki Krogerus wrote:
> This will prepare the device connection API for connections
> described in firmware.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> ---
> include/linux/device.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 90224e75ade4..a964a0d614fa 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -753,11 +753,17 @@ struct device_dma_parameters {
>
> /**
> * 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 registeded
for your next version:) 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;
>
cheers.
--
~Randy
Hello!
On 10/24/2018 06:05 PM, Heikki Krogerus wrote:
> When the connections are defined in firmware, struct
> device_connection will have the fwnode member pointing to
> the device node (struct fwnode_handle) of the requested
> device, and the endpoint will not be used at all in that
> case.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/class.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 5db0593ca0bd..fe6f3a932a88 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -204,15 +204,28 @@ static void typec_altmode_put_partner(struct altmode *altmode)
> put_device(&adev->dev);
> }
>
> -static int __typec_port_match(struct device *dev, const void *name)
> +static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
> +{
> + return dev_name(fwnode) == fwnode;
dev_name(dev), maybe?
[...]
MBR, Sergei
On Wed, Oct 24, 2018 at 08:07:09PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 10/24/2018 06:05 PM, Heikki Krogerus wrote:
>
> > When the connections are defined in firmware, struct
> > device_connection will have the fwnode member pointing to
> > the device node (struct fwnode_handle) of the requested
> > device, and the endpoint will not be used at all in that
> > case.
> >
> > Signed-off-by: Heikki Krogerus <[email protected]>
> > ---
> > drivers/usb/typec/class.c | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 5db0593ca0bd..fe6f3a932a88 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -204,15 +204,28 @@ static void typec_altmode_put_partner(struct altmode *altmode)
> > put_device(&adev->dev);
> > }
> >
> > -static int __typec_port_match(struct device *dev, const void *name)
> > +static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
> > +{
> > + return dev_name(fwnode) == fwnode;
>
> dev_name(dev), maybe?
Yes.
thanks,
--
heikki
On Wed, Oct 24, 2018 at 08:33:53AM -0700, Randy Dunlap wrote:
> On 10/24/18 8:05 AM, Heikki Krogerus wrote:
> > This will prepare the device connection API for connections
> > described in firmware.
> >
> > Signed-off-by: Heikki Krogerus <[email protected]>
> > ---
> > include/linux/device.h | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 90224e75ade4..a964a0d614fa 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -753,11 +753,17 @@ struct device_dma_parameters {
> >
> > /**
> > * 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 registeded
>
> for your next version:) registered
OK.
thanks,
--
heikki
On Thu, Oct 25, 2018 at 10:27:20AM +0300, Heikki Krogerus wrote:
> On Wed, Oct 24, 2018 at 08:07:09PM +0300, Sergei Shtylyov wrote:
> > Hello!
> >
> > On 10/24/2018 06:05 PM, Heikki Krogerus wrote:
> >
> > > When the connections are defined in firmware, struct
> > > device_connection will have the fwnode member pointing to
> > > the device node (struct fwnode_handle) of the requested
> > > device, and the endpoint will not be used at all in that
> > > case.
> > >
> > > Signed-off-by: Heikki Krogerus <[email protected]>
> > > ---
> > > drivers/usb/typec/class.c | 19 ++++++++++++++++---
> > > 1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > > index 5db0593ca0bd..fe6f3a932a88 100644
> > > --- a/drivers/usb/typec/class.c
> > > +++ b/drivers/usb/typec/class.c
> > > @@ -204,15 +204,28 @@ static void typec_altmode_put_partner(struct altmode *altmode)
> > > put_device(&adev->dev);
> > > }
> > >
> > > -static int __typec_port_match(struct device *dev, const void *name)
> > > +static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
> > > +{
> > > + return dev_name(fwnode) == fwnode;
> >
> > dev_name(dev), maybe?
Actually:
dev_fwnode(dev) == fwnode;
Cheers,
--
heikki
Hi Heikki,
Heikki Krogerus <[email protected]> 于2018年10月24日周三 下午11:06写道：
>
> When the connections are defined in firmware, struct
> device_connection will have the fwnode member pointing to
> the device node (struct fwnode_handle) of the requested
> device, and the endpoint will not be used at all in that
> case.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/common/roles.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
> index 99116af07f1d..bb52e006d203 100644
> --- a/drivers/usb/common/roles.c
> +++ b/drivers/usb/common/roles.c
> @@ -8,6 +8,7 @@
> */
>
> #include <linux/usb/role.h>
> +#include <linux/property.h>
> #include <linux/device.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> @@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
> }
> EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
>
> -static int __switch_match(struct device *dev, const void *name)
> +static int switch_fwnode_match(struct device *dev, const void *fwnode)
> +{
> + return dev_fwnode(dev) == fwnode;
Seems this should be dev_fwnode(dev->parent) == fwnode;
The role switch is the child dev of the caller of usb_role_switch_register().
Li Jun
> +}
> +
> +static int switch_name_match(struct device *dev, const void *name)
> {
> return !strcmp((const char *)name, dev_name(dev));
> }
> @@ -94,8 +100,12 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
> {
> struct device *dev;
>
> - dev = class_find_device(role_class, NULL, con->endpoint[ep],
> - __switch_match);
> + if (con->fwnode)
> + dev = class_find_device(role_class, NULL, con->fwnode,
> + switch_fwnode_match);
> + else
> + dev = class_find_device(role_class, NULL, con->endpoint[ep],
> + switch_name_match);
>
> return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
> }
> --
> 2.19.1
>
On Tue, Jan 22, 2019 at 01:40:02PM +0800, Jun Li wrote:
> Hi Heikki,
> Heikki Krogerus <[email protected]> 于2018年10月24日周三 下午11:06写道：
> >
> > When the connections are defined in firmware, struct
> > device_connection will have the fwnode member pointing to
> > the device node (struct fwnode_handle) of the requested
> > device, and the endpoint will not be used at all in that
> > case.
> >
> > Signed-off-by: Heikki Krogerus <[email protected]>
> > ---
> > drivers/usb/common/roles.c | 16 +++++++++++++---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
> > index 99116af07f1d..bb52e006d203 100644
> > --- a/drivers/usb/common/roles.c
> > +++ b/drivers/usb/common/roles.c
> > @@ -8,6 +8,7 @@
> > */
> >
> > #include <linux/usb/role.h>
> > +#include <linux/property.h>
> > #include <linux/device.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > @@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
> > }
> > EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
> >
> > -static int __switch_match(struct device *dev, const void *name)
> > +static int switch_fwnode_match(struct device *dev, const void *fwnode)
> > +{
> > + return dev_fwnode(dev) == fwnode;
>
> Seems this should be dev_fwnode(dev->parent) == fwnode;
> The role switch is the child dev of the caller of usb_role_switch_register().
Indeed.
thanks,
--
heikki