2020-12-13 08:13:27

by Sam Protsenko

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

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


2020-12-13 11:30:52

by Sam Protsenko

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

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 <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
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

2020-12-13 18:43:14

by Andy Shevchenko

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

On Sun, Dec 13, 2020 at 5:27 AM Sam Protsenko
<[email protected]> 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