| Message ID | 20240901040658.157425-12-swboyd@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | platform/chrome: Add DT USB/DP muxing/topology support | expand |
On Sat, Aug 31, 2024 at 09:06:49PM -0700, Stephen Boyd wrote: > When a single DT node has a graph connected to more than one > usb-c-connector node we can't differentiate which typec switch > registered for the device is associated with the USB connector because > the devcon matcher code assumes a 1:1 relationship between remote node > and typec switch. Furthermore, we don't have a #typec-switch-cells > property so there can only be one node per typec switch. > > Support multiple USB typec switches exposed by one node by passing the > remote endpoint node in addition to the remote node to the devcon > matcher function (devcon_match_fn_t). With this change, typec switch > drivers can register switches with the device node pointer for a graph > endpoint so that they can support more than one typec switch if > necessary. Either way, a DT property like 'mode-switch' is always in the > graph's parent node and not in the endpoint node. > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Daniel Scally <djrscally@gmail.com> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Ivan Orlov <ivan.orlov0322@gmail.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: Conor Dooley <conor+dt@kernel.org> > Cc: <devicetree@vger.kernel.org> > Cc: <linux-usb@vger.kernel.org> > Cc: <linux-acpi@vger.kernel.org> > Cc: Pin-yen Lin <treapking@chromium.org> Is it possible to move these Cc:s after --- line below? > Signed-off-by: Stephen Boyd <swboyd@chromium.org> ... > /** > * devcon_match_fn_t - device connection match function > * @fwnode: Remote connection's device node > + * @endpoint: Remote connection's endpoint node > * @con_id: Identifier for the connection > * @data: Match function caller specific data > * > * Implement a callback with this function signature to search a fwnode's > * connections for a match with a function like device_connection_find_match(). > * This function will be called possibly multiple times, once for each > - * connection. The match function should inspect the @fwnode to look for a > - * match. The @con_id and @data provided are the same as the @con_id and @data > - * arguments passed to the functions that take a devcon_match_fn_t argument. > + * connection. The match function should inspect the connection's @fwnode > + * and/or @endpoint to look for a match. The @con_id and @data provided are the > + * same as the @con_id and @data arguments passed to the functions that take a > + * devcon_match_fn_t argument. So, struct fwnode_handle is a single-linked list. Can we utilise that instead of adding a new parameter? I.o.w. do those objects (@fwnode and @endpoint) have anything in common and can be chained? > * Note: This function can be called multiple times. What does this mean? Is it idempotent? Or what is the effect of being called multiple times? > * > * Return: Pointer to match or NULL if no match found. > */
Quoting Andy Shevchenko (2024-09-02 04:12:49) > On Sat, Aug 31, 2024 at 09:06:49PM -0700, Stephen Boyd wrote: > > Is it possible to move these Cc:s after --- line below? Ok. > > > /** > > * devcon_match_fn_t - device connection match function > > * @fwnode: Remote connection's device node > > + * @endpoint: Remote connection's endpoint node > > * @con_id: Identifier for the connection > > * @data: Match function caller specific data > > * > > * Implement a callback with this function signature to search a fwnode's > > * connections for a match with a function like device_connection_find_match(). > > * This function will be called possibly multiple times, once for each > > - * connection. The match function should inspect the @fwnode to look for a > > - * match. The @con_id and @data provided are the same as the @con_id and @data > > - * arguments passed to the functions that take a devcon_match_fn_t argument. > > + * connection. The match function should inspect the connection's @fwnode > > + * and/or @endpoint to look for a match. The @con_id and @data provided are the > > + * same as the @con_id and @data arguments passed to the functions that take a > > + * devcon_match_fn_t argument. > > So, struct fwnode_handle is a single-linked list. Can we utilise that instead > of adding a new parameter? I.o.w. do those objects (@fwnode and @endpoint) have > anything in common and can be chained? No, we can't use that. We need to know which endpoint in the remote fwnode is connected to the fwnode we're searching from. This is how we know which typec mux structure is associated with which type-c port so we can drive DP there. We might have two endpoints connected to the same fwnode and then we wouldn't be able to differentiate the endpoint and the typec mux to configure. > > > * Note: This function can be called multiple times. > > What does this mean? Is it idempotent? Or what is the effect of being called > multiple times? I've removed this note now.
diff --git a/drivers/base/property.c b/drivers/base/property.c index 837d77e3af2b..621de33f2956 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -1284,6 +1284,7 @@ static unsigned int fwnode_graph_devcon_matches(const struct fwnode_handle *fwno { struct fwnode_handle *node; struct fwnode_handle *ep; + struct fwnode_handle *remote_ep; unsigned int count = 0; void *ret; @@ -1299,7 +1300,9 @@ static unsigned int fwnode_graph_devcon_matches(const struct fwnode_handle *fwno continue; } - ret = match(node, con_id, data); + remote_ep = fwnode_graph_get_remote_endpoint(ep); + ret = match(node, remote_ep, con_id, data); + fwnode_handle_put(remote_ep); fwnode_handle_put(node); if (ret) { if (matches) @@ -1329,7 +1332,7 @@ static unsigned int fwnode_devcon_matches(const struct fwnode_handle *fwnode, if (IS_ERR(node)) break; - ret = match(node, NULL, data); + ret = match(node, NULL, NULL, data); fwnode_handle_put(node); if (ret) { if (matches) diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c index d7aa913ceb8a..d8bd5071d9d8 100644 --- a/drivers/usb/roles/class.c +++ b/drivers/usb/roles/class.c @@ -121,8 +121,8 @@ 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(const struct fwnode_handle *fwnode, const char *id, - void *data) +static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const struct fwnode_handle *endpoint, + const char *id, void *data) { struct device *dev; diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c index f420185e36e3..9ddb6ec54325 100644 --- a/drivers/usb/typec/mux.c +++ b/drivers/usb/typec/mux.c @@ -33,6 +33,7 @@ static int switch_fwnode_match(struct device *dev, const void *fwnode) } static void *typec_switch_match(const struct fwnode_handle *fwnode, + const struct fwnode_handle *endpoint, const char *id, void *data) { struct device *dev; @@ -55,6 +56,9 @@ static void *typec_switch_match(const struct fwnode_handle *fwnode, */ dev = class_find_device(&typec_mux_class, NULL, fwnode, switch_fwnode_match); + if (!dev) + dev = class_find_device(&typec_mux_class, NULL, endpoint, + switch_fwnode_match); return dev ? to_typec_switch_dev(dev) : ERR_PTR(-EPROBE_DEFER); } @@ -299,6 +303,7 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode) } static void *typec_mux_match(const struct fwnode_handle *fwnode, + const struct fwnode_handle *endpoint, const char *id, void *data) { struct device *dev; @@ -316,6 +321,9 @@ static void *typec_mux_match(const struct fwnode_handle *fwnode, dev = class_find_device(&typec_mux_class, NULL, fwnode, mux_fwnode_match); + if (!dev) + dev = class_find_device(&typec_mux_class, NULL, endpoint, + mux_fwnode_match); return dev ? to_typec_mux_dev(dev) : ERR_PTR(-EPROBE_DEFER); } diff --git a/drivers/usb/typec/retimer.c b/drivers/usb/typec/retimer.c index b519fcf358ca..ee4e6312c2d9 100644 --- a/drivers/usb/typec/retimer.c +++ b/drivers/usb/typec/retimer.c @@ -22,7 +22,9 @@ static int retimer_fwnode_match(struct device *dev, const void *fwnode) return is_typec_retimer(dev) && device_match_fwnode(dev, fwnode); } -static void *typec_retimer_match(const struct fwnode_handle *fwnode, const char *id, void *data) +static void *typec_retimer_match(const struct fwnode_handle *fwnode, + const struct fwnode_handle *endpoint, + const char *id, void *data) { struct device *dev; @@ -31,6 +33,9 @@ static void *typec_retimer_match(const struct fwnode_handle *fwnode, const char dev = class_find_device(&retimer_class, NULL, fwnode, retimer_fwnode_match); + if (!dev) + dev = class_find_device(&retimer_class, NULL, endpoint, + retimer_fwnode_match); return dev ? to_typec_retimer(dev) : ERR_PTR(-EPROBE_DEFER); } diff --git a/include/linux/property.h b/include/linux/property.h index 797b1eeda7d2..72a89cd410db 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -510,22 +510,25 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode, /** * devcon_match_fn_t - device connection match function * @fwnode: Remote connection's device node + * @endpoint: Remote connection's endpoint node * @con_id: Identifier for the connection * @data: Match function caller specific data * * Implement a callback with this function signature to search a fwnode's * connections for a match with a function like device_connection_find_match(). * This function will be called possibly multiple times, once for each - * connection. The match function should inspect the @fwnode to look for a - * match. The @con_id and @data provided are the same as the @con_id and @data - * arguments passed to the functions that take a devcon_match_fn_t argument. + * connection. The match function should inspect the connection's @fwnode + * and/or @endpoint to look for a match. The @con_id and @data provided are the + * same as the @con_id and @data arguments passed to the functions that take a + * devcon_match_fn_t argument. * * Note: This function can be called multiple times. * * Return: Pointer to match or NULL if no match found. */ typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode, - const char *con_id, void *data); + const struct fwnode_handle *endpoint, + const char *id, void *data); void *fwnode_connection_find_match(const struct fwnode_handle *fwnode, const char *con_id, void *data,
When a single DT node has a graph connected to more than one usb-c-connector node we can't differentiate which typec switch registered for the device is associated with the USB connector because the devcon matcher code assumes a 1:1 relationship between remote node and typec switch. Furthermore, we don't have a #typec-switch-cells property so there can only be one node per typec switch. Support multiple USB typec switches exposed by one node by passing the remote endpoint node in addition to the remote node to the devcon matcher function (devcon_match_fn_t). With this change, typec switch drivers can register switches with the device node pointer for a graph endpoint so that they can support more than one typec switch if necessary. Either way, a DT property like 'mode-switch' is always in the graph's parent node and not in the endpoint node. Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Daniel Scally <djrscally@gmail.com> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Vinod Koul <vkoul@kernel.org> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Ivan Orlov <ivan.orlov0322@gmail.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> Cc: Conor Dooley <conor+dt@kernel.org> Cc: <devicetree@vger.kernel.org> Cc: <linux-usb@vger.kernel.org> Cc: <linux-acpi@vger.kernel.org> Cc: Pin-yen Lin <treapking@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/base/property.c | 7 +++++-- drivers/usb/roles/class.c | 4 ++-- drivers/usb/typec/mux.c | 8 ++++++++ drivers/usb/typec/retimer.c | 7 ++++++- include/linux/property.h | 11 +++++++---- 5 files changed, 28 insertions(+), 9 deletions(-)