Hi,
This small patchset tries to address issue with broken DT extcon property
in case of USB controller - DWC3. It exposes similar problem as in proposed
USB connector bindings[1] - extcon device is required by devices not always
connected directly to extcon device. Here we have:
DWC3 -> USB-PHY -> MUIC -> USB-connector
^
MHL-bridge---------------------^
More details and proposition of generic solution in first patch
[1]: https://marc.info/?i=20180131134435.12216-1-a.hajda%40samsung.com
Regards
Andrzej
Andrzej Hajda (2):
USB: dwc3: get extcon device by OF graph bindings
arm64: dts: exynos: add OF graph between USB-PHY and MUIC
.../boot/dts/exynos/exynos5433-tm2-common.dtsi | 18 +++++++++-
drivers/usb/dwc3/drd.c | 41 +++++++++++++++-------
2 files changed, 45 insertions(+), 14 deletions(-)
--
2.15.1
OF graph describes USB data lanes between USB-PHY and respective MUIC.
Since graph is present and DWC driver can use it to get extcon, obsolete
extcon property can be removed.
Signed-off-by: Andrzej Hajda <[email protected]>
---
arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 427ff861b441..b20b42659206 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -828,6 +828,17 @@
};
};
};
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ muic_to_usb: endpoint {
+ remote-endpoint = <&usb_to_muic>;
+ };
+ };
};
};
@@ -1242,12 +1253,17 @@
&usbdrd_dwc3 {
dr_mode = "otg";
- extcon = <&muic>;
};
&usbdrd30_phy {
vbus-supply = <&safeout1_reg>;
status = "okay";
+
+ port {
+ usb_to_muic: endpoint {
+ remote-endpoint = <&muic_to_usb>;
+ };
+ };
};
&xxti {
--
2.15.1
extcon device is used to detect host/device connection. Since extcon
OF property is deprecated, alternative method should be added.
This method uses OF graph bindings to locate extcon.
Signed-off-by: Andrzej Hajda <[email protected]>
---
Hi all,
This patch implements alternative method to get extcon from DWC3.
The code works but is hacky, as DWC3 must traverse different DT nodes
to get extcon, in case of TM2 it is USB-PHY and MUIC, but other
platforms can have different paths.
I would be glad if it can be merged as is for now, but additional work
must be done to make it generic.
I guess on DT binding side it is OK. So the problem should be addressed
in the code.
My rough idea is to implement kind of extcon aliases/forwarder mechanism,
ie. USB-PHY will expect on its output remote port extcon, and it should register
extcon-forwarder pointing to this extcon. This way DWC3 can look for the extcon
on its PHY phandle, and it will receive via forwarding mechanism extcon
exposed by MUIC.
As I said this is rough idea for discussion, other propositions are welcome.
Regards
Andrzej
---
drivers/usb/dwc3/drd.c | 41 ++++++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index cc8ab9a8e9d2..eee2eca3d513 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -8,6 +8,7 @@
*/
#include <linux/extcon.h>
+#include <linux/of_graph.h>
#include "debug.h"
#include "core.h"
@@ -38,24 +39,38 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
return NOTIFY_DONE;
}
+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;
+
+ if (of_property_read_bool(dev->of_node, "extcon"))
+ return extcon_get_edev_by_phandle(dwc->dev, 0);
+
+ np_phy = of_parse_phandle(dev->of_node, "phys", 0);
+ np_conn = of_graph_get_remote_node(np_phy, -1, -1);
+ edev = extcon_get_edev_by_of_node(np_conn);
+ of_node_put(np_conn);
+ of_node_put(np_phy);
+
+ return edev;
+}
+
int dwc3_drd_init(struct dwc3 *dwc)
{
int ret;
- if (dwc->dev->of_node) {
- if (of_property_read_bool(dwc->dev->of_node, "extcon"))
- dwc->edev = extcon_get_edev_by_phandle(dwc->dev, 0);
-
- if (IS_ERR(dwc->edev))
- return PTR_ERR(dwc->edev);
+ dwc->edev = dwc3_get_extcon(dwc);
+ if (IS_ERR(dwc->edev))
+ return PTR_ERR(dwc->edev);
- dwc->edev_nb.notifier_call = dwc3_drd_notifier;
- ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
- &dwc->edev_nb);
- if (ret < 0) {
- dev_err(dwc->dev, "couldn't register cable notifier\n");
- return ret;
- }
+ dwc->edev_nb.notifier_call = dwc3_drd_notifier;
+ ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
+ &dwc->edev_nb);
+ if (ret < 0) {
+ dev_err(dwc->dev, "couldn't register cable notifier\n");
+ return ret;
}
dwc3_drd_update(dwc);
--
2.15.1
On Wed, Jan 31, 2018 at 4:57 PM, Andrzej Hajda <[email protected]> wrote:
> OF graph describes USB data lanes between USB-PHY and respective MUIC.
> Since graph is present and DWC driver can use it to get extcon, obsolete
> extcon property can be removed.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index 427ff861b441..b20b42659206 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -828,6 +828,17 @@
> };
> };
> };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + muic_to_usb: endpoint {
> + remote-endpoint = <&usb_to_muic>;
> + };
> + };
> };
> };
>
> @@ -1242,12 +1253,17 @@
>
> &usbdrd_dwc3 {
> dr_mode = "otg";
> - extcon = <&muic>;
Look ok for me.
Does this depend on #1 patch for DWC?
Best regards,
Krzysztof
On 01.02.2018 08:50, Krzysztof Kozlowski wrote:
> On Wed, Jan 31, 2018 at 4:57 PM, Andrzej Hajda <[email protected]> wrote:
>> OF graph describes USB data lanes between USB-PHY and respective MUIC.
>> Since graph is present and DWC driver can use it to get extcon, obsolete
>> extcon property can be removed.
>>
>> Signed-off-by: Andrzej Hajda <[email protected]>
>> ---
>> arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> index 427ff861b441..b20b42659206 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> @@ -828,6 +828,17 @@
>> };
>> };
>> };
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + muic_to_usb: endpoint {
>> + remote-endpoint = <&usb_to_muic>;
>> + };
>> + };
>> };
>> };
>>
>> @@ -1242,12 +1253,17 @@
>>
>> &usbdrd_dwc3 {
>> dr_mode = "otg";
>> - extcon = <&muic>;
> Look ok for me.
>
> Does this depend on #1 patch for DWC?
Yes, it depends. #1 adds code to find extcon using graph, without it
dwc3 won't be able to work in dual-role mode.
Alternatively, I could split this patch into two parts:
2a. adding graph bindings.
2b. removing extcon property.
In such case only 2b depends on #1.
Regards
Andrzej
>
> Best regards,
> Krzysztof
>
>
>
On Thu, Feb 1, 2018 at 9:19 AM, Andrzej Hajda <[email protected]> wrote:
> On 01.02.2018 08:50, Krzysztof Kozlowski wrote:
>> On Wed, Jan 31, 2018 at 4:57 PM, Andrzej Hajda <[email protected]> wrote:
>>> OF graph describes USB data lanes between USB-PHY and respective MUIC.
>>> Since graph is present and DWC driver can use it to get extcon, obsolete
>>> extcon property can be removed.
>>>
>>> Signed-off-by: Andrzej Hajda <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 18 +++++++++++++++++-
>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>> index 427ff861b441..b20b42659206 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>>> @@ -828,6 +828,17 @@
>>> };
>>> };
>>> };
>>> +
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + muic_to_usb: endpoint {
>>> + remote-endpoint = <&usb_to_muic>;
>>> + };
>>> + };
>>> };
>>> };
>>>
>>> @@ -1242,12 +1253,17 @@
>>>
>>> &usbdrd_dwc3 {
>>> dr_mode = "otg";
>>> - extcon = <&muic>;
>> Look ok for me.
>>
>> Does this depend on #1 patch for DWC?
>
> Yes, it depends. #1 adds code to find extcon using graph, without it
> dwc3 won't be able to work in dual-role mode.
> Alternatively, I could split this patch into two parts:
> 2a. adding graph bindings.
> 2b. removing extcon property.
> In such case only 2b depends on #1.
Without splitting, this patch would have to wait till #1 gets into
mainline (so probably till v4.17-rc1 because we are in merge window
already). I am fine with this split-approach so only the cleanup patch
would get postponed. Up to you.
Best regards,
Krzysztof
On 31.01.2018 16:57, Andrzej Hajda wrote:
> extcon device is used to detect host/device connection. Since extcon
> OF property is deprecated, alternative method should be added.
> This method uses OF graph bindings to locate extcon.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
Ping, 3.5 months passed.
Regards
Andrzej
> ---
> Hi all,
>
> This patch implements alternative method to get extcon from DWC3.
> The code works but is hacky, as DWC3 must traverse different DT nodes
> to get extcon, in case of TM2 it is USB-PHY and MUIC, but other
> platforms can have different paths.
> I would be glad if it can be merged as is for now, but additional work
> must be done to make it generic.
> I guess on DT binding side it is OK. So the problem should be addressed
> in the code.
> My rough idea is to implement kind of extcon aliases/forwarder mechanism,
> ie. USB-PHY will expect on its output remote port extcon, and it should register
> extcon-forwarder pointing to this extcon. This way DWC3 can look for the extcon
> on its PHY phandle, and it will receive via forwarding mechanism extcon
> exposed by MUIC.
> As I said this is rough idea for discussion, other propositions are welcome.
>
> Regards
> Andrzej
> ---
> drivers/usb/dwc3/drd.c | 41 ++++++++++++++++++++++++++++-------------
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index cc8ab9a8e9d2..eee2eca3d513 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -8,6 +8,7 @@
> */
>
> #include <linux/extcon.h>
> +#include <linux/of_graph.h>
>
> #include "debug.h"
> #include "core.h"
> @@ -38,24 +39,38 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +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;
> +
> + if (of_property_read_bool(dev->of_node, "extcon"))
> + return extcon_get_edev_by_phandle(dwc->dev, 0);
> +
> + np_phy = of_parse_phandle(dev->of_node, "phys", 0);
> + np_conn = of_graph_get_remote_node(np_phy, -1, -1);
> + edev = extcon_get_edev_by_of_node(np_conn);
> + of_node_put(np_conn);
> + of_node_put(np_phy);
> +
> + return edev;
> +}
> +
> int dwc3_drd_init(struct dwc3 *dwc)
> {
> int ret;
>
> - if (dwc->dev->of_node) {
> - if (of_property_read_bool(dwc->dev->of_node, "extcon"))
> - dwc->edev = extcon_get_edev_by_phandle(dwc->dev, 0);
> -
> - if (IS_ERR(dwc->edev))
> - return PTR_ERR(dwc->edev);
> + dwc->edev = dwc3_get_extcon(dwc);
> + if (IS_ERR(dwc->edev))
> + return PTR_ERR(dwc->edev);
>
> - dwc->edev_nb.notifier_call = dwc3_drd_notifier;
> - ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> - &dwc->edev_nb);
> - if (ret < 0) {
> - dev_err(dwc->dev, "couldn't register cable notifier\n");
> - return ret;
> - }
> + dwc->edev_nb.notifier_call = dwc3_drd_notifier;
> + ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> + &dwc->edev_nb);
> + if (ret < 0) {
> + dev_err(dwc->dev, "couldn't register cable notifier\n");
> + return ret;
> }
>
> dwc3_drd_update(dwc);
Andrzej Hajda <[email protected]> writes:
> extcon device is used to detect host/device connection. Since extcon
> OF property is deprecated, alternative method should be added.
> This method uses OF graph bindings to locate extcon.
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> Hi all,
>
> This patch implements alternative method to get extcon from DWC3.
> The code works but is hacky, as DWC3 must traverse different DT nodes
> to get extcon, in case of TM2 it is USB-PHY and MUIC, but other
> platforms can have different paths.
> I would be glad if it can be merged as is for now, but additional work
> must be done to make it generic.
> I guess on DT binding side it is OK. So the problem should be addressed
> in the code.
> My rough idea is to implement kind of extcon aliases/forwarder mechanism,
> ie. USB-PHY will expect on its output remote port extcon, and it should register
> extcon-forwarder pointing to this extcon. This way DWC3 can look for the extcon
> on its PHY phandle, and it will receive via forwarding mechanism extcon
> exposed by MUIC.
> As I said this is rough idea for discussion, other propositions are welcome.
>
> Regards
> Andrzej
I need someone from devicetree to review and ack patch2 before I can
apply them. Either way, this doesn't apply:
checking file drivers/usb/dwc3/drd.c
Hunk #1 FAILED at 8.
Hunk #2 FAILED at 38.
2 out of 2 hunks FAILED
--
balbi
On Tue, May 15, 2018 at 9:26 AM, Felipe Balbi <[email protected]> wrote:
> Andrzej Hajda <[email protected]> writes:
>
>> extcon device is used to detect host/device connection. Since extcon
>> OF property is deprecated, alternative method should be added.
>> This method uses OF graph bindings to locate extcon.
>>
>> Signed-off-by: Andrzej Hajda <[email protected]>
>> ---
>> Hi all,
>>
>> This patch implements alternative method to get extcon from DWC3.
>> The code works but is hacky, as DWC3 must traverse different DT nodes
>> to get extcon, in case of TM2 it is USB-PHY and MUIC, but other
>> platforms can have different paths.
>> I would be glad if it can be merged as is for now, but additional work
>> must be done to make it generic.
>> I guess on DT binding side it is OK. So the problem should be addressed
>> in the code.
>> My rough idea is to implement kind of extcon aliases/forwarder mechanism,
>> ie. USB-PHY will expect on its output remote port extcon, and it should register
>> extcon-forwarder pointing to this extcon. This way DWC3 can look for the extcon
>> on its PHY phandle, and it will receive via forwarding mechanism extcon
>> exposed by MUIC.
>> As I said this is rough idea for discussion, other propositions are welcome.
>>
>> Regards
>> Andrzej
>
> I need someone from devicetree to review and ack patch2 before I can
> apply them. Either way, this doesn't apply:
The DTS patch will go through arm-soc tree, I'll take it. The DTS
patches are independent from drivers and shall not usually go through
regular trees.
Best regards,
Krzysztof