This series adds USB3 OTG controller reset for rk3399 Type-C PHY, and use the
reset to hold the whole USB3 OTG controller in reset state to keep the PIPE
power state in P2 before initializing Type-C PHY, it's useful to avoid waiting
for PHY PMA and PIPE ready timeout.
William Wu (3):
dt-bindings: phy: phy-rockchip-typec: add usb3 otg reset
arm64: dts: rockchip: add USB3 OTG reset for Type-C PHY on rk3399
phy: rockchip-typec: reset USB3 controller before initializing PHY
.../devicetree/bindings/phy/phy-rockchip-typec.txt | 12 +++++++-----
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++----
drivers/phy/rockchip/phy-rockchip-typec.c | 22 ++++++++++++++++++++--
3 files changed, 33 insertions(+), 11 deletions(-)
--
2.0.0
According to the RK3399 TRM, for Type-C USB start-up sequence,
we need to hold the whole USB 3.0 OTG controller in reset state
to keep the PIPE power state in P2 while initializing PHY. This
is because when initialize the Type-C PHY for USB3, we need to
configure the PHY and PMA for the selected mode of operation,
and wait for the PMA and PIPE ready, if the USB3 OTG controller
isn't in P2 state, it may cause waiting timeout.
Without this patch, waiting for the PMA and PIPE ready timeout
issue easily happens when we shutdown the Logic on RK3399 and
do the suspend/resume stress test.
Signed-off-by: William Wu <[email protected]>
---
drivers/phy/rockchip/phy-rockchip-typec.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index ee85fa0..68a5840 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -372,6 +372,7 @@ struct rockchip_typec_phy {
struct reset_control *uphy_rst;
struct reset_control *pipe_rst;
struct reset_control *tcphy_rst;
+ struct reset_control *otg_rst;
struct rockchip_usb3phy_port_cfg port_cfgs;
/* mutex to protect access to individual PHYs */
struct mutex lock;
@@ -841,10 +842,16 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
if (tcphy->mode == new_mode)
goto unlock_ret;
+ ret = reset_control_assert(tcphy->otg_rst);
+ if (ret < 0) {
+ dev_err(tcphy->dev, "failed to assert otg reset: %d\n", ret);
+ goto unlock_ret;
+ }
+
if (tcphy->mode == MODE_DISCONNECT) {
ret = tcphy_phy_init(tcphy, new_mode);
if (ret)
- goto unlock_ret;
+ goto unlock_deassert;
}
/* wait TCPHY for pipe ready */
@@ -852,7 +859,7 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
regmap_read(tcphy->grf_regs, reg->offset, &val);
if (!(val & BIT(reg->enable_bit))) {
tcphy->mode |= new_mode & (MODE_DFP_USB | MODE_UFP_USB);
- goto unlock_ret;
+ goto unlock_deassert;
}
usleep_range(10, 20);
}
@@ -862,6 +869,11 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
ret = -ETIMEDOUT;
+unlock_deassert:
+ ret = reset_control_deassert(tcphy->otg_rst);
+ if (ret < 0)
+ dev_err(tcphy->dev, "failed to deassert otg reset: %d\n", ret);
+
unlock_ret:
mutex_unlock(&tcphy->lock);
return ret;
@@ -1066,6 +1078,12 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
return PTR_ERR(tcphy->tcphy_rst);
}
+ tcphy->otg_rst = devm_reset_control_get(dev, "usb3-otg");
+ if (IS_ERR(tcphy->otg_rst)) {
+ dev_err(dev, "no otg_rst reset control found\n");
+ return PTR_ERR(tcphy->otg_rst);
+ }
+
return 0;
}
--
2.0.0
Add USB3 OTG reset for Type-C PHY. It can be used to hold the USB3
OTG controller in reset state before initializing the Type-C PHY.
Signed-off-by: William Wu <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d340b58a..4e89d00 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1377,8 +1377,9 @@
power-domains = <&power RK3399_PD_TCPD0>;
resets = <&cru SRST_UPHY0>,
<&cru SRST_UPHY0_PIPE_L00>,
- <&cru SRST_P_UPHY0_TCPHY>;
- reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
+ <&cru SRST_P_UPHY0_TCPHY>,
+ <&cru SRST_A_USB3_OTG0>;
+ reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
rockchip,grf = <&grf>;
rockchip,typec-conn-dir = <0xe580 0 16>;
rockchip,usb3tousb2-en = <0xe580 3 19>;
@@ -1406,8 +1407,9 @@
power-domains = <&power RK3399_PD_TCPD1>;
resets = <&cru SRST_UPHY1>,
<&cru SRST_UPHY1_PIPE_L00>,
- <&cru SRST_P_UPHY1_TCPHY>;
- reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
+ <&cru SRST_P_UPHY1_TCPHY>,
+ <&cru SRST_A_USB3_OTG1>;
+ reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
rockchip,grf = <&grf>;
rockchip,typec-conn-dir = <0xe58c 0 16>;
rockchip,usb3tousb2-en = <0xe58c 3 19>;
--
2.0.0
This patch adds USB3 OTG reset property for rk3399 Type-C PHY
to hold the USB3 controller in reset state.
Signed-off-by: William Wu <[email protected]>
---
Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
index 6ea867e..db2902e 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
@@ -13,7 +13,7 @@ Required properties:
- assigned-clock-rates : the phy core clk frequency, shall be: 50000000
- resets : a list of phandle + reset specifier pairs
- reset-names : string reset name, must be:
- "uphy", "uphy-pipe", "uphy-tcphy"
+ "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg"
- extcon : extcon specifier for the Power Delivery
Note, there are 2 type-c phys for RK3399, and they are almost identical, except
@@ -56,8 +56,9 @@ Example:
assigned-clock-rates = <50000000>;
resets = <&cru SRST_UPHY0>,
<&cru SRST_UPHY0_PIPE_L00>,
- <&cru SRST_P_UPHY0_TCPHY>;
- reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
+ <&cru SRST_P_UPHY0_TCPHY>,
+ <&cru SRST_A_USB3_OTG0>;
+ reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
rockchip,typec-conn-dir = <0xe580 0 16>;
rockchip,usb3tousb2-en = <0xe580 3 19>;
rockchip,external-psm = <0xe588 14 30>;
@@ -84,8 +85,9 @@ Example:
assigned-clock-rates = <50000000>;
resets = <&cru SRST_UPHY1>,
<&cru SRST_UPHY1_PIPE_L00>,
- <&cru SRST_P_UPHY1_TCPHY>;
- reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
+ <&cru SRST_P_UPHY1_TCPHY>,
+ <&cru SRST_A_USB3_OTG1>;
+ reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
rockchip,typec-conn-dir = <0xe58c 0 16>;
rockchip,usb3tousb2-en = <0xe58c 3 19>;
rockchip,external-psm = <0xe594 14 30>;
--
2.0.0
+ Enric
On Fri, Jan 12, 2018 at 06:08:22PM +0800, William Wu wrote:
> This patch adds USB3 OTG reset property for rk3399 Type-C PHY
> to hold the USB3 controller in reset state.
>
> Signed-off-by: William Wu <[email protected]>
> ---
I was going back and forth on this, since at one point this binding was
merged but had no enabled users...but now I see Heiko has queued up some
of Enric's work for 4.16, and it uses the existing binding.
So, if this reset is added, it should be optional.
Brian
> Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
> index 6ea867e..db2902e 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
> @@ -13,7 +13,7 @@ Required properties:
> - assigned-clock-rates : the phy core clk frequency, shall be: 50000000
> - resets : a list of phandle + reset specifier pairs
> - reset-names : string reset name, must be:
> - "uphy", "uphy-pipe", "uphy-tcphy"
> + "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg"
> - extcon : extcon specifier for the Power Delivery
>
> Note, there are 2 type-c phys for RK3399, and they are almost identical, except
> @@ -56,8 +56,9 @@ Example:
> assigned-clock-rates = <50000000>;
> resets = <&cru SRST_UPHY0>,
> <&cru SRST_UPHY0_PIPE_L00>,
> - <&cru SRST_P_UPHY0_TCPHY>;
> - reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
> + <&cru SRST_P_UPHY0_TCPHY>,
> + <&cru SRST_A_USB3_OTG0>;
> + reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
> rockchip,typec-conn-dir = <0xe580 0 16>;
> rockchip,usb3tousb2-en = <0xe580 3 19>;
> rockchip,external-psm = <0xe588 14 30>;
> @@ -84,8 +85,9 @@ Example:
> assigned-clock-rates = <50000000>;
> resets = <&cru SRST_UPHY1>,
> <&cru SRST_UPHY1_PIPE_L00>,
> - <&cru SRST_P_UPHY1_TCPHY>;
> - reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
> + <&cru SRST_P_UPHY1_TCPHY>,
> + <&cru SRST_A_USB3_OTG1>;
> + reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
> rockchip,typec-conn-dir = <0xe58c 0 16>;
> rockchip,usb3tousb2-en = <0xe58c 3 19>;
> rockchip,external-psm = <0xe594 14 30>;
> --
> 2.0.0
>
>
2018-01-17 23:07 GMT+01:00 Brian Norris <[email protected]>:
> + Enric
>
> On Fri, Jan 12, 2018 at 06:08:22PM +0800, William Wu wrote:
>> This patch adds USB3 OTG reset property for rk3399 Type-C PHY
>> to hold the USB3 controller in reset state.
>>
>> Signed-off-by: William Wu <[email protected]>
>> ---
>
> I was going back and forth on this, since at one point this binding was
> merged but had no enabled users...but now I see Heiko has queued up some
> of Enric's work for 4.16, and it uses the existing binding.
>
> So, if this reset is added, it should be optional.
>
As Brian said commit 06c47e6286d5 'usb: dwc3: of-simple: Add support
to get resets for the device' introduced the support to get the resets
from dwc3-of-simple and the queued commit 'b7e63d95c14d arm64: dts:
rockchip: add reset property for dwc3 controllers on rk3399' started
using it. Without the latest I get errors like this doing bind/unbind
tests.
dwc3: probe of fe900000.dwc3 failed with error -110
I just tested these series on top of mainline, I reverted my patch
because otherwise two drivers are requesting the same reset and fails,
and I did some of the bind/unbind test. They just worked fine, and
seems that this is right way, so this makes me think some questions.
Should 'b7e63d95c14d arm64: dts: rockchip: add reset property for dwc3
controllers on rk3399' removed for 4.16? That's a question for Heiko I
guess, if it's removed we will have usb broken meanwhile these patches
doesn't land. If we don't remove the patch we will need to introduce a
new patch in this series that reverts the first patch.
Is commit 06c47e6286d5 'usb: dwc3: of-simple: Add support to get
resets for the device' really needed ? Seems that I was the only user
of it.
Anyway, these patches looks good to me and are
Tested-by: Enric Balletbo i Serra <[email protected]>
> Brian
>
>> Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>> index 6ea867e..db2902e 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
>> @@ -13,7 +13,7 @@ Required properties:
>> - assigned-clock-rates : the phy core clk frequency, shall be: 50000000
>> - resets : a list of phandle + reset specifier pairs
>> - reset-names : string reset name, must be:
>> - "uphy", "uphy-pipe", "uphy-tcphy"
>> + "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg"
>> - extcon : extcon specifier for the Power Delivery
>>
>> Note, there are 2 type-c phys for RK3399, and they are almost identical, except
>> @@ -56,8 +56,9 @@ Example:
>> assigned-clock-rates = <50000000>;
>> resets = <&cru SRST_UPHY0>,
>> <&cru SRST_UPHY0_PIPE_L00>,
>> - <&cru SRST_P_UPHY0_TCPHY>;
>> - reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
>> + <&cru SRST_P_UPHY0_TCPHY>,
>> + <&cru SRST_A_USB3_OTG0>;
>> + reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
>> rockchip,typec-conn-dir = <0xe580 0 16>;
>> rockchip,usb3tousb2-en = <0xe580 3 19>;
>> rockchip,external-psm = <0xe588 14 30>;
>> @@ -84,8 +85,9 @@ Example:
>> assigned-clock-rates = <50000000>;
>> resets = <&cru SRST_UPHY1>,
>> <&cru SRST_UPHY1_PIPE_L00>,
>> - <&cru SRST_P_UPHY1_TCPHY>;
>> - reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
>> + <&cru SRST_P_UPHY1_TCPHY>,
>> + <&cru SRST_A_USB3_OTG1>;
>> + reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
>> rockchip,typec-conn-dir = <0xe58c 0 16>;
>> rockchip,usb3tousb2-en = <0xe58c 3 19>;
>> rockchip,external-psm = <0xe594 14 30>;
>> --
>> 2.0.0
>>
>>
2018-01-12 11:08 GMT+01:00 William Wu <[email protected]>:
> Add USB3 OTG reset for Type-C PHY. It can be used to hold the USB3
> OTG controller in reset state before initializing the Type-C PHY.
>
> Signed-off-by: William Wu <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index d340b58a..4e89d00 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1377,8 +1377,9 @@
> power-domains = <&power RK3399_PD_TCPD0>;
> resets = <&cru SRST_UPHY0>,
> <&cru SRST_UPHY0_PIPE_L00>,
> - <&cru SRST_P_UPHY0_TCPHY>;
> - reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
> + <&cru SRST_P_UPHY0_TCPHY>,
> + <&cru SRST_A_USB3_OTG0>;
> + reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
> rockchip,grf = <&grf>;
> rockchip,typec-conn-dir = <0xe580 0 16>;
> rockchip,usb3tousb2-en = <0xe580 3 19>;
> @@ -1406,8 +1407,9 @@
> power-domains = <&power RK3399_PD_TCPD1>;
> resets = <&cru SRST_UPHY1>,
> <&cru SRST_UPHY1_PIPE_L00>,
> - <&cru SRST_P_UPHY1_TCPHY>;
> - reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
> + <&cru SRST_P_UPHY1_TCPHY>,
> + <&cru SRST_A_USB3_OTG1>;
> + reset-names = "uphy", "uphy-pipe", "uphy-tcphy", "usb3-otg";
> rockchip,grf = <&grf>;
> rockchip,typec-conn-dir = <0xe58c 0 16>;
> rockchip,usb3tousb2-en = <0xe58c 3 19>;
> --
> 2.0.0
>
>
Tested-by: Enric Balletbo i Serra <[email protected]>
2018-01-12 11:08 GMT+01:00 William Wu <[email protected]>:
> According to the RK3399 TRM, for Type-C USB start-up sequence,
> we need to hold the whole USB 3.0 OTG controller in reset state
> to keep the PIPE power state in P2 while initializing PHY. This
> is because when initialize the Type-C PHY for USB3, we need to
> configure the PHY and PMA for the selected mode of operation,
> and wait for the PMA and PIPE ready, if the USB3 OTG controller
> isn't in P2 state, it may cause waiting timeout.
>
> Without this patch, waiting for the PMA and PIPE ready timeout
> issue easily happens when we shutdown the Logic on RK3399 and
> do the suspend/resume stress test.
>
> Signed-off-by: William Wu <[email protected]>
> ---
> drivers/phy/rockchip/phy-rockchip-typec.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index ee85fa0..68a5840 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -372,6 +372,7 @@ struct rockchip_typec_phy {
> struct reset_control *uphy_rst;
> struct reset_control *pipe_rst;
> struct reset_control *tcphy_rst;
> + struct reset_control *otg_rst;
> struct rockchip_usb3phy_port_cfg port_cfgs;
> /* mutex to protect access to individual PHYs */
> struct mutex lock;
> @@ -841,10 +842,16 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
> if (tcphy->mode == new_mode)
> goto unlock_ret;
>
> + ret = reset_control_assert(tcphy->otg_rst);
> + if (ret < 0) {
> + dev_err(tcphy->dev, "failed to assert otg reset: %d\n", ret);
> + goto unlock_ret;
> + }
> +
> if (tcphy->mode == MODE_DISCONNECT) {
> ret = tcphy_phy_init(tcphy, new_mode);
> if (ret)
> - goto unlock_ret;
> + goto unlock_deassert;
> }
>
> /* wait TCPHY for pipe ready */
> @@ -852,7 +859,7 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
> regmap_read(tcphy->grf_regs, reg->offset, &val);
> if (!(val & BIT(reg->enable_bit))) {
> tcphy->mode |= new_mode & (MODE_DFP_USB | MODE_UFP_USB);
> - goto unlock_ret;
> + goto unlock_deassert;
> }
> usleep_range(10, 20);
> }
> @@ -862,6 +869,11 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
>
> ret = -ETIMEDOUT;
>
> +unlock_deassert:
> + ret = reset_control_deassert(tcphy->otg_rst);
> + if (ret < 0)
> + dev_err(tcphy->dev, "failed to deassert otg reset: %d\n", ret);
> +
> unlock_ret:
> mutex_unlock(&tcphy->lock);
> return ret;
> @@ -1066,6 +1078,12 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> return PTR_ERR(tcphy->tcphy_rst);
> }
>
> + tcphy->otg_rst = devm_reset_control_get(dev, "usb3-otg");
> + if (IS_ERR(tcphy->otg_rst)) {
> + dev_err(dev, "no otg_rst reset control found\n");
> + return PTR_ERR(tcphy->otg_rst);
> + }
> +
> return 0;
> }
>
> --
> 2.0.0
>
>
Tested-by: Enric Balletbo i Serra <[email protected]>
On Thu, Jan 18, 2018 at 06:20:09PM +0100, Enric Balletbo Serra wrote:
> As Brian said commit 06c47e6286d5 'usb: dwc3: of-simple: Add support
> to get resets for the device' introduced the support to get the resets
> from dwc3-of-simple and the queued commit 'b7e63d95c14d arm64: dts:
> rockchip: add reset property for dwc3 controllers on rk3399' started
> using it. Without the latest I get errors like this doing bind/unbind
> tests.
>
> dwc3: probe of fe900000.dwc3 failed with error -110
>
> I just tested these series on top of mainline, I reverted my patch
> because otherwise two drivers are requesting the same reset and fails,
> and I did some of the bind/unbind test. They just worked fine, and
> seems that this is right way, so this makes me think some questions.
Actually, this was intended to coexist with DWC3 optionally controlling
the same reset. It was written before the reset framework was rewritten
to have shared and exclusive resets. Should this be rewritten to use
shared resets? We'd have to modify both dwc3 core and the PHY driver.
> Should 'b7e63d95c14d arm64: dts: rockchip: add reset property for dwc3
> controllers on rk3399' removed for 4.16? That's a question for Heiko I
> guess, if it's removed we will have usb broken meanwhile these patches
> doesn't land. If we don't remove the patch we will need to introduce a
> new patch in this series that reverts the first patch.
If we don't make this shared, I suppose Rockchip should rewrite this
series and rebase on Heiko's tree, where the DTSI change should just
remove the reset from DWC3 and add it to the PHY node at the same time.
That would be an atomic, non-breaking change.
> Is commit 06c47e6286d5 'usb: dwc3: of-simple: Add support to get
> resets for the device' really needed ? Seems that I was the only user
> of it.
I don't personally know the history of this one. I just noticed that you
started using it to resolve a problem that was implemented differently
in our downstream tree.
> Anyway, these patches looks good to me and are
>
> Tested-by: Enric Balletbo i Serra <[email protected]>
Awesome.
Brian
On Thu, Jan 18, 2018 at 09:47:50AM -0800, Brian Norris wrote:
> On Thu, Jan 18, 2018 at 06:20:09PM +0100, Enric Balletbo Serra wrote:
> > As Brian said commit 06c47e6286d5 'usb: dwc3: of-simple: Add support
> > to get resets for the device' introduced the support to get the resets
> > from dwc3-of-simple and the queued commit 'b7e63d95c14d arm64: dts:
> > rockchip: add reset property for dwc3 controllers on rk3399' started
> > using it. Without the latest I get errors like this doing bind/unbind
> > tests.
> >
> > dwc3: probe of fe900000.dwc3 failed with error -110
> >
> > I just tested these series on top of mainline, I reverted my patch
> > because otherwise two drivers are requesting the same reset and fails,
> > and I did some of the bind/unbind test. They just worked fine, and
> > seems that this is right way, so this makes me think some questions.
>
> Actually, this was intended to coexist with DWC3 optionally controlling
> the same reset. It was written before the reset framework was rewritten
> to have shared and exclusive resets. Should this be rewritten to use
> shared resets? We'd have to modify both dwc3 core and the PHY driver.
Seems like abuse of DT to me. If you need to control the controller's
reset from the phy driver, then get the reset out of the controller
node. The phy node should describe the connections to the phy.
Rob
Hi Wulf,
Am Montag, 22. Januar 2018, 12:33:05 CET schrieb wlf:
> 在 2018年01月20日 05:49, Rob Herring 写道:
> > On Thu, Jan 18, 2018 at 09:47:50AM -0800, Brian Norris wrote:
> >> On Thu, Jan 18, 2018 at 06:20:09PM +0100, Enric Balletbo Serra wrote:
> >>> As Brian said commit 06c47e6286d5 'usb: dwc3: of-simple: Add support
> >>> to get resets for the device' introduced the support to get the resets
> >>> from dwc3-of-simple and the queued commit 'b7e63d95c14d arm64: dts:
> >>> rockchip: add reset property for dwc3 controllers on rk3399' started
> >>> using it. Without the latest I get errors like this doing bind/unbind
> >>> tests.
> >>>
> >>> dwc3: probe of fe900000.dwc3 failed with error -110
> >>>
> >>> I just tested these series on top of mainline, I reverted my patch
> >>> because otherwise two drivers are requesting the same reset and fails,
> >>> and I did some of the bind/unbind test. They just worked fine, and
> >>> seems that this is right way, so this makes me think some questions.
> >> Actually, this was intended to coexist with DWC3 optionally controlling
> >> the same reset. It was written before the reset framework was rewritten
> >> to have shared and exclusive resets. Should this be rewritten to use
> >> shared resets? We'd have to modify both dwc3 core and the PHY driver.
> > Seems like abuse of DT to me. If you need to control the controller's
> > reset from the phy driver, then get the reset out of the controller
> > node. The phy node should describe the connections to the phy.
> I try to get the reset out of the controller, but I don't find a good
> way to get the reset ofthe controller associated with the given phy
> device node. Is there an API like theof_usb_get_dr_mode_by_phy() to
> get 'dr_mode' of the controller?
I guess the easiest way would be taking the of_usb_get_dr_mode_by_phy()
function move the part above the "finish" label into a separate function
like of_usb_get_node_by_phy (or possibly move that to an even more general
place as it is not usb-related at all) and use that function in
of_usb_get_dr_mode_by_phy() and also use it to get the reset you want via
something like:
node = of_usb_get_node_by_phy(...);
rst = of_reset_control_get(node, ...);
> And we're trying to find another method to fix the RK3399 tcphy power
> on fail issue.If we get another proper method, we may not need these
> phy patch series.
Did you find any different solution for that yet?
Heiko
Dear Heiko,
在 2018年02月11日 03:55, Heiko Stuebner 写道:
> Hi Wulf,
>
> Am Montag, 22. Januar 2018, 12:33:05 CET schrieb wlf:
>> 在 2018年01月20日 05:49, Rob Herring 写道:
>>> On Thu, Jan 18, 2018 at 09:47:50AM -0800, Brian Norris wrote:
>>>> On Thu, Jan 18, 2018 at 06:20:09PM +0100, Enric Balletbo Serra wrote:
>>>>> As Brian said commit 06c47e6286d5 'usb: dwc3: of-simple: Add support
>>>>> to get resets for the device' introduced the support to get the resets
>>>>> from dwc3-of-simple and the queued commit 'b7e63d95c14d arm64: dts:
>>>>> rockchip: add reset property for dwc3 controllers on rk3399' started
>>>>> using it. Without the latest I get errors like this doing bind/unbind
>>>>> tests.
>>>>>
>>>>> dwc3: probe of fe900000.dwc3 failed with error -110
>>>>>
>>>>> I just tested these series on top of mainline, I reverted my patch
>>>>> because otherwise two drivers are requesting the same reset and fails,
>>>>> and I did some of the bind/unbind test. They just worked fine, and
>>>>> seems that this is right way, so this makes me think some questions.
>>>> Actually, this was intended to coexist with DWC3 optionally controlling
>>>> the same reset. It was written before the reset framework was rewritten
>>>> to have shared and exclusive resets. Should this be rewritten to use
>>>> shared resets? We'd have to modify both dwc3 core and the PHY driver.
>>> Seems like abuse of DT to me. If you need to control the controller's
>>> reset from the phy driver, then get the reset out of the controller
>>> node. The phy node should describe the connections to the phy.
>> I try to get the reset out of the controller, but I don't find a good
>> way to get the reset ofthe controller associated with the given phy
>> device node. Is there an API like theof_usb_get_dr_mode_by_phy() to
>> get 'dr_mode' of the controller?
> I guess the easiest way would be taking the of_usb_get_dr_mode_by_phy()
> function move the part above the "finish" label into a separate function
> like of_usb_get_node_by_phy (or possibly move that to an even more general
> place as it is not usb-related at all) and use that function in
> of_usb_get_dr_mode_by_phy() and also use it to get the reset you want via
> something like:
>
> node = of_usb_get_node_by_phy(...);
> rst = of_reset_control_get(node, ...);
Thanks for your good suggestion. I think it's a good way to get the
reset of the
controller by phy.
>> And we're trying to find another method to fix the RK3399 tcphy power
>> on fail issue.If we get another proper method, we may not need these
>> phy patch series.
> Did you find any different solution for that yet?
>
> Heiko
Yes, finally, we have found another way to fix the RK3399 tcphy usb power
on fail issue. And Enric Balletbo i Serra <Enric Balletbo i Serra> has
submitted
new patch series, refer to the following patches:
https://patchwork.kernel.org/patch/10207261/
https://patchwork.kernel.org/patch/10207267/
https://patchwork.kernel.org/patch/10207263/
So my patches can be abandoned.
>