[PATCH v3 0/2] usb: dwc3: drd: Check graph presence for extcon

[PATCH v3 0/2] usb: dwc3: drd: Check graph presence for extcon

[Sam Protsenko]

dwc3 shows error message on probe if port node wasn't found in PHY
controller node. But that is actually a valid case for the role switch
mode and OTG mode. This patch series makes sure to hide that error
message and also does a bit of refactoring for that code. Similar patch
(for different subsystem) already exists in kernel tree:
commit ea5bc3b15e0f ("drm/of: Make drm_of_find_panel_or_bridge() to
check graph's presence"), and the whole `of_graph_is_present()' function
was implemented exactly for this kind of situation.

In previous submission (v2) it was a single patch. But in offline
discussion with Andy Shevchenko it was decided it's better to split it
into two patches in order to provide the minimal change for further
possible backporting, and do all style related changes on top of it in
second patch.

Sam Protsenko (2):
  usb: dwc3: drd: Avoid error when extcon is missing
  usb: dwc3: drd: Improve dwc3_get_extcon() style

 drivers/usb/dwc3/drd.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

-- 
2.29.2

[PATCH v3 1/2] usb: dwc3: drd: Avoid error when extcon is missing

[Sam Protsenko]

If "port" node is missing in PHY controller node, dwc3_get_extcon()
isn't able to find extcon device. This is perfectly fine in case when
"usb-role-switch" or OTG is used, but next misleading error message is
printed in that case, from of_graph_get_remote_node():

    OF: graph: no port node found in /phy@1234abcd

Avoid printing that message by checking if port node exists in PHY node
before calling of_graph_get_remote_node().

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v3:
  - Split patch into two patches: logic diff and style diff

 drivers/usb/dwc3/drd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 3e1c1aacf002..312a4d060e80 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -463,7 +463,10 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 	}
 
 	np_phy = of_parse_phandle(dev->of_node, "phys", 0);
-	np_conn = of_graph_get_remote_node(np_phy, -1, -1);
+	if (of_graph_is_present(np_phy))
+		np_conn = of_graph_get_remote_node(np_phy, -1, -1);
+	else
+		np_conn = NULL;
 
 	if (np_conn)
 		edev = extcon_find_edev_by_node(np_conn);
-- 
2.29.2

[PATCH v3 2/2] usb: dwc3: drd: Improve dwc3_get_extcon() style

[Sam Protsenko]

Commit c73b41955ee4 ("usb: dwc3: drd: Avoid error when extcon is
missing") changed the code flow in dwc3_get_extcon() function, leading
to unnecessary if-branch. This patch does housekeeping by reworking the
code for obtaining extcon device from the "port" node. While at it, add
the  comment from mentioned code block, explaining how checking the port
availability helps to avoid the misleading error.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v3:
  - Split patch into two patches: logic diff and style diff

 drivers/usb/dwc3/drd.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 312a4d060e80..eaf389d3f3c5 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -441,8 +441,8 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
 static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 {
 	struct device *dev = dwc->dev;
-	struct device_node *np_phy, *np_conn;
-	struct extcon_dev *edev;
+	struct device_node *np_phy;
+	struct extcon_dev *edev = NULL;
 	const char *name;
 
 	if (device_property_read_bool(dev, "extcon"))
@@ -462,18 +462,22 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 		return edev;
 	}
 
+	/*
+	 * Try to get extcon device from USB PHY controller's "port" node.
+	 * Check if it has the "port" node first, to avoid printing the error
+	 * message from underlying code, as it's a valid case: extcon device
+	 * (and "port" node) may be missing in case of "usb-role-switch" or OTG
+	 * mode.
+	 */
 	np_phy = of_parse_phandle(dev->of_node, "phys", 0);
-	if (of_graph_is_present(np_phy))
-		np_conn = of_graph_get_remote_node(np_phy, -1, -1);
-	else
-		np_conn = NULL;
+	if (of_graph_is_present(np_phy)) {
+		struct device_node *np_conn;
 
-	if (np_conn)
-		edev = extcon_find_edev_by_node(np_conn);
-	else
-		edev = NULL;
-
-	of_node_put(np_conn);
+		np_conn = of_graph_get_remote_node(np_phy, -1, -1);
+		if (np_conn)
+			edev = extcon_find_edev_by_node(np_conn);
+		of_node_put(np_conn);
+	}
 	of_node_put(np_phy);
 
 	return edev;
-- 
2.29.2

Re: [PATCH v3 1/2] usb: dwc3: drd: Avoid error when extcon is missing

[Andy Shevchenko]

On Sun, Dec 13, 2020 at 5:22 AM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> If "port" node is missing in PHY controller node, dwc3_get_extcon()
> isn't able to find extcon device. This is perfectly fine in case when

devices

> "usb-role-switch" or OTG is used, but next misleading error message is
> printed in that case, from of_graph_get_remote_node():
>
>     OF: graph: no port node found in /phy@1234abcd
>
> Avoid printing that message by checking if port node exists in PHY node

if the port

> before calling of_graph_get_remote_node().

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> Changes in v3:
>   - Split patch into two patches: logic diff and style diff
>
>  drivers/usb/dwc3/drd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 3e1c1aacf002..312a4d060e80 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -463,7 +463,10 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
>         }
>
>         np_phy = of_parse_phandle(dev->of_node, "phys", 0);
> -       np_conn = of_graph_get_remote_node(np_phy, -1, -1);
> +       if (of_graph_is_present(np_phy))
> +               np_conn = of_graph_get_remote_node(np_phy, -1, -1);
> +       else
> +               np_conn = NULL;
>
>         if (np_conn)
>                 edev = extcon_find_edev_by_node(np_conn);
> --
> 2.29.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] usb: dwc3: drd: Improve dwc3_get_extcon() style

[Andy Shevchenko]

On Sun, Dec 13, 2020 at 5:27 AM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> Commit c73b41955ee4 ("usb: dwc3: drd: Avoid error when extcon is

The previous change ("...

(because we don't know the actual commit id)

> missing") changed the code flow in dwc3_get_extcon() function, leading
> to unnecessary if-branch. This patch does housekeeping by reworking the
> code for obtaining extcon device from the "port" node. While at it, add

an extcon

> the  comment from mentioned code block, explaining how checking the port
> availability helps to avoid the misleading error.

...

> +       /*
> +        * Try to get extcon device from USB PHY controller's "port" node.

an extcon
the USB

> +        * Check if it has the "port" node first, to avoid printing the error
> +        * message from underlying code, as it's a valid case: extcon device
> +        * (and "port" node) may be missing in case of "usb-role-switch" or OTG
> +        * mode.
> +        */

To me it sounds like this should be in the previous patch.

-- 
With Best Regards,
Andy Shevchenko