[PATCH v2 0/1] fix USB host in HS mode on Allwinner H3 and newer

[PATCH v2 0/1] fix USB host in HS mode on Allwinner H3 and newer

[Evgeny Boger]

This patch makes USB host work reliably in HS mode on Allwinner A40i (R40)
and some other SoCs by fixing the way the internal PHY registers are
written.

Changes in v2:
  As suggested by Andre, I've added the detailed description of the nature
  of the issue. Appart from commit message, nothing is changed compared to
  v1.

Evgeny Boger (1):
  phy: sun4i-usb: fix phy write on H3 and newer

 drivers/phy/allwinner/phy-sun4i-usb.c | 44 ++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 5 deletions(-)

-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Evgeny Boger]

We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
in our custom A40i-based design. What we observe is that after several
attempts USB device would fail to enumerate on EHCI port (HS) and will be
enumerated instead on OHCI (FS, 12Mb/s) only:

[    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
[    6.818008] usb 1-1: device not accepting address 5, error -71
[    6.823868] usb usb1-port1: unable to enumerate USB device
[    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
[    7.575045] usb 3-1: not running at top speed; connect to a high speed hub

On some boards one of the ports would work in high-speed mode, but
on most of the boards all three USB ports would only work in FS mode.

At the same time, USB work flawlessly in high-speed mode in vendor kernel.

Looking for the differences in USB code, we found the issue with USB PHY
register initialization. Basically, USB PHY driver sets a couple of
internal undocumented PHY registers to the predefined constants. These PHY
registers are accessed in a very (and I mean VERY) weird way by shifting
register addresses and values bit-by-bit. This access method was slightly
changed starting from H3 SoC, according to the BSP source for different
SoCs.

As a result, mainline PHY driver won't set these PHY registers properly
resulting in unreliable enumeration in high-speed mode.

We don't know whether this issue will result in broken HS mode on all
affected SoCs or instead the A40i is an unfortunate exception. What we
indeed verified, is that BSPs for all affected SoCs write these registers
properly while mainline kernel don't. We also were able to reproduce the
USB issue on a couple of A40i boards from other vendors, so we are pretty
sure these registers have to always be properly set, regardlress of
a hardware layout.

The proposed patch is tested on A40i-based Wiren Board 7 building
automation controller. More details are below.

On older SoCs (prior to H3) PHY register are accessed by manipulating
the common register for all PHYs. PHY index is specified by pulsing
usbc bit.

Newer SoCs leave the access procedure mostly unchanged, the
difference being that the latch registers are separate for each PHY.

Additionally, accessing USB PHY registers is only possible if phy0 is
routed to musb IP instead of HCI.

Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.

On A83t, H6, H616, T507 and probably on more recent hardware,
these PHY registers are not used in vendor BSP.
So don't set v2 flag for these even newer SoCs as a precaution.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c | 44 ++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 788dd5cdbb7d..cf10e385f199 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -119,6 +119,7 @@ struct sun4i_usb_phy_cfg {
 	bool dedicated_clocks;
 	bool enable_pmu_unk1;
 	bool phy0_dual_route;
+	bool phy_reg_access_v2;
 	int missing_phys;
 };
 
@@ -192,13 +193,38 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
 				int len)
 {
 	struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
-	u32 temp, usbc_bit = BIT(phy->index * 2);
+	u32 otgctl_val, temp, usbc_bit;
 	void __iomem *phyctl = phy_data->base + phy_data->cfg->phyctl_offset;
+	void __iomem *phyctl_latch;
 	unsigned long flags;
 	int i;
 
 	spin_lock_irqsave(&phy_data->reg_lock, flags);
 
+	/* On older SoCs (prior to H3) PHY register are accessed by manipulating the
+	 * common register for all PHYs. PHY index is specified by pulsing usbc bit.
+	 * Newer SoCs leave the access procedure mostly unchanged, the difference
+	 * being that the latch registers are separate for each PHY.
+	 */
+	if (phy_data->cfg->phy_reg_access_v2) {
+		if (phy->index == 0)
+			phyctl_latch = phy_data->base + phy_data->cfg->phyctl_offset;
+		else
+			phyctl_latch = phy->pmu + phy_data->cfg->phyctl_offset;
+		usbc_bit = 1;
+
+		/* Accessing USB PHY registers is only possible if phy0 is routed to musb.
+		 * As it's not clear whether is this related to actual PHY
+		 * routing or rather the hardware is just reusing the same bit,
+		 * don't check phy0_dual_route here.
+		 */
+		otgctl_val = readl(phy_data->base + REG_PHY_OTGCTL);
+		writel(otgctl_val | OTGCTL_ROUTE_MUSB, phy_data->base + REG_PHY_OTGCTL);
+	} else {
+		phyctl_latch = phyctl;
+		usbc_bit = BIT(phy->index * 2);
+	}
+
 	if (phy_data->cfg->phyctl_offset == REG_PHYCTL_A33) {
 		/* SoCs newer than A33 need us to set phyctl to 0 explicitly */
 		writel(0, phyctl);
@@ -224,17 +250,21 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
 		writeb(temp, phyctl);
 
 		/* pulse usbc_bit */
-		temp = readb(phyctl);
+		temp = readb(phyctl_latch);
 		temp |= usbc_bit;
-		writeb(temp, phyctl);
+		writeb(temp, phyctl_latch);
 
-		temp = readb(phyctl);
+		temp = readb(phyctl_latch);
 		temp &= ~usbc_bit;
-		writeb(temp, phyctl);
+		writeb(temp, phyctl_latch);
 
 		data >>= 1;
 	}
 
+	/* Restore PHY routing and the rest of OTGCTL */
+	if (phy_data->cfg->phy_reg_access_v2)
+		writel(otgctl_val, phy_data->base + REG_PHY_OTGCTL);
+
 	spin_unlock_irqrestore(&phy_data->reg_lock, flags);
 }
 
@@ -927,6 +957,7 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
 	.dedicated_clocks = true,
 	.enable_pmu_unk1 = true,
 	.phy0_dual_route = true,
+	.phy_reg_access_v2 = true,
 };
 
 static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = {
@@ -937,6 +968,7 @@ static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = {
 	.dedicated_clocks = true,
 	.enable_pmu_unk1 = true,
 	.phy0_dual_route = true,
+	.phy_reg_access_v2 = true,
 };
 
 static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
@@ -947,6 +979,7 @@ static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
 	.dedicated_clocks = true,
 	.enable_pmu_unk1 = true,
 	.phy0_dual_route = true,
+	.phy_reg_access_v2 = true,
 };
 
 static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = {
@@ -957,6 +990,7 @@ static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = {
 	.dedicated_clocks = true,
 	.enable_pmu_unk1 = true,
 	.phy0_dual_route = true,
+	.phy_reg_access_v2 = true,
 };
 
 static const struct sun4i_usb_phy_cfg sun50i_h6_cfg = {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 4970 bytes --]

Hi,

On Tue, Jan 11, 2022 at 07:51:53PM +0300, Evgeny Boger wrote:
> We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
> in our custom A40i-based design. What we observe is that after several
> attempts USB device would fail to enumerate on EHCI port (HS) and will be
> enumerated instead on OHCI (FS, 12Mb/s) only:
> 
> [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
> [    6.818008] usb 1-1: device not accepting address 5, error -71
> [    6.823868] usb usb1-port1: unable to enumerate USB device
> [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
> [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
> 
> On some boards one of the ports would work in high-speed mode, but
> on most of the boards all three USB ports would only work in FS mode.
> 
> At the same time, USB work flawlessly in high-speed mode in vendor kernel.
> 
> Looking for the differences in USB code, we found the issue with USB PHY
> register initialization. Basically, USB PHY driver sets a couple of
> internal undocumented PHY registers to the predefined constants. These PHY
> registers are accessed in a very (and I mean VERY) weird way by shifting
> register addresses and values bit-by-bit. This access method was slightly
> changed starting from H3 SoC, according to the BSP source for different
> SoCs.
> 
> As a result, mainline PHY driver won't set these PHY registers properly
> resulting in unreliable enumeration in high-speed mode.
> 
> We don't know whether this issue will result in broken HS mode on all
> affected SoCs or instead the A40i is an unfortunate exception. What we
> indeed verified, is that BSPs for all affected SoCs write these registers
> properly while mainline kernel don't. We also were able to reproduce the
> USB issue on a couple of A40i boards from other vendors, so we are pretty
> sure these registers have to always be properly set, regardlress of
> a hardware layout.
> 
> The proposed patch is tested on A40i-based Wiren Board 7 building
> automation controller. More details are below.
> 
> On older SoCs (prior to H3) PHY register are accessed by manipulating
> the common register for all PHYs. PHY index is specified by pulsing
> usbc bit.
> 
> Newer SoCs leave the access procedure mostly unchanged, the
> difference being that the latch registers are separate for each PHY.
> 
> Additionally, accessing USB PHY registers is only possible if phy0 is
> routed to musb IP instead of HCI.
> 
> Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
> R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
> 
> On A83t, H6, H616, T507 and probably on more recent hardware,
> these PHY registers are not used in vendor BSP.
> So don't set v2 flag for these even newer SoCs as a precaution.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

This should probably be sent to stable, and have a Fixes tag?

> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c | 44 ++++++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 788dd5cdbb7d..cf10e385f199 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -119,6 +119,7 @@ struct sun4i_usb_phy_cfg {
>  	bool dedicated_clocks;
>  	bool enable_pmu_unk1;
>  	bool phy0_dual_route;
> +	bool phy_reg_access_v2;
>  	int missing_phys;
>  };
>  
> @@ -192,13 +193,38 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>  				int len)
>  {
>  	struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
> -	u32 temp, usbc_bit = BIT(phy->index * 2);
> +	u32 otgctl_val, temp, usbc_bit;
>  	void __iomem *phyctl = phy_data->base + phy_data->cfg->phyctl_offset;
> +	void __iomem *phyctl_latch;
>  	unsigned long flags;
>  	int i;
>  
>  	spin_lock_irqsave(&phy_data->reg_lock, flags);
>  
> +	/* On older SoCs (prior to H3) PHY register are accessed by manipulating the
> +	 * common register for all PHYs. PHY index is specified by pulsing usbc bit.
> +	 * Newer SoCs leave the access procedure mostly unchanged, the difference
> +	 * being that the latch registers are separate for each PHY.
> +	 */

Multi-line comments need to start with an empty line

> +	if (phy_data->cfg->phy_reg_access_v2) {
> +		if (phy->index == 0)
> +			phyctl_latch = phy_data->base + phy_data->cfg->phyctl_offset;
> +		else
> +			phyctl_latch = phy->pmu + phy_data->cfg->phyctl_offset;
> +		usbc_bit = 1;
> +
> +		/* Accessing USB PHY registers is only possible if phy0 is routed to musb.
> +		 * As it's not clear whether is this related to actual PHY
> +		 * routing or rather the hardware is just reusing the same bit,
> +		 * don't check phy0_dual_route here.
> +		 */

Ditto

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Evgeny Boger]

Hi Maxime!
12.01.2022 11:42, Maxime Ripard пишет:
> Hi,
>
> On Tue, Jan 11, 2022 at 07:51:53PM +0300, Evgeny Boger wrote:
>> We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
>> in our custom A40i-based design. What we observe is that after several
>> attempts USB device would fail to enumerate on EHCI port (HS) and will be
>> enumerated instead on OHCI (FS, 12Mb/s) only:
>>
>> [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
>> [    6.818008] usb 1-1: device not accepting address 5, error -71
>> [    6.823868] usb usb1-port1: unable to enumerate USB device
>> [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
>> [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
>>
>> On some boards one of the ports would work in high-speed mode, but
>> on most of the boards all three USB ports would only work in FS mode.
>>
>> At the same time, USB work flawlessly in high-speed mode in vendor kernel.
>>
>> Looking for the differences in USB code, we found the issue with USB PHY
>> register initialization. Basically, USB PHY driver sets a couple of
>> internal undocumented PHY registers to the predefined constants. These PHY
>> registers are accessed in a very (and I mean VERY) weird way by shifting
>> register addresses and values bit-by-bit. This access method was slightly
>> changed starting from H3 SoC, according to the BSP source for different
>> SoCs.
>>
>> As a result, mainline PHY driver won't set these PHY registers properly
>> resulting in unreliable enumeration in high-speed mode.
>>
>> We don't know whether this issue will result in broken HS mode on all
>> affected SoCs or instead the A40i is an unfortunate exception. What we
>> indeed verified, is that BSPs for all affected SoCs write these registers
>> properly while mainline kernel don't. We also were able to reproduce the
>> USB issue on a couple of A40i boards from other vendors, so we are pretty
>> sure these registers have to always be properly set, regardlress of
>> a hardware layout.
>>
>> The proposed patch is tested on A40i-based Wiren Board 7 building
>> automation controller. More details are below.
>>
>> On older SoCs (prior to H3) PHY register are accessed by manipulating
>> the common register for all PHYs. PHY index is specified by pulsing
>> usbc bit.
>>
>> Newer SoCs leave the access procedure mostly unchanged, the
>> difference being that the latch registers are separate for each PHY.
>>
>> Additionally, accessing USB PHY registers is only possible if phy0 is
>> routed to musb IP instead of HCI.
>>
>> Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
>> R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
>>
>> On A83t, H6, H616, T507 and probably on more recent hardware,
>> these PHY registers are not used in vendor BSP.
>> So don't set v2 flag for these even newer SoCs as a precaution.
>>
>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> This should probably be sent to stable, and have a Fixes tag?

well, phy register writes never worked on H3 SoC and newer, so there is 
no specific commit which broke it. So nothing to put to Fixes: tag.

I'm not sure it qualifies for stable too. Citing guidelines:

 >It must be obviously correct and tested

I was only able to test it on handful of A40i boards and on one A20 board

 >It must fix a real bug that bothers people

this bug for certain bothers *us*, but our users won't benefit from 
porting this fix to stable.
I didn't find reports on this bug from other people.
I think unfortunately not many people are using mainline kernel on these 
SoCs.


>
>> ---
>>   drivers/phy/allwinner/phy-sun4i-usb.c | 44 ++++++++++++++++++++++++---
>>   1 file changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
>> index 788dd5cdbb7d..cf10e385f199 100644
>> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
>> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
>> @@ -119,6 +119,7 @@ struct sun4i_usb_phy_cfg {
>>   	bool dedicated_clocks;
>>   	bool enable_pmu_unk1;
>>   	bool phy0_dual_route;
>> +	bool phy_reg_access_v2;
>>   	int missing_phys;
>>   };
>>   
>> @@ -192,13 +193,38 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
>>   				int len)
>>   {
>>   	struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
>> -	u32 temp, usbc_bit = BIT(phy->index * 2);
>> +	u32 otgctl_val, temp, usbc_bit;
>>   	void __iomem *phyctl = phy_data->base + phy_data->cfg->phyctl_offset;
>> +	void __iomem *phyctl_latch;
>>   	unsigned long flags;
>>   	int i;
>>   
>>   	spin_lock_irqsave(&phy_data->reg_lock, flags);
>>   
>> +	/* On older SoCs (prior to H3) PHY register are accessed by manipulating the
>> +	 * common register for all PHYs. PHY index is specified by pulsing usbc bit.
>> +	 * Newer SoCs leave the access procedure mostly unchanged, the difference
>> +	 * being that the latch registers are separate for each PHY.
>> +	 */
> Multi-line comments need to start with an empty line
>
>> +	if (phy_data->cfg->phy_reg_access_v2) {
>> +		if (phy->index == 0)
>> +			phyctl_latch = phy_data->base + phy_data->cfg->phyctl_offset;
>> +		else
>> +			phyctl_latch = phy->pmu + phy_data->cfg->phyctl_offset;
>> +		usbc_bit = 1;
>> +
>> +		/* Accessing USB PHY registers is only possible if phy0 is routed to musb.
>> +		 * As it's not clear whether is this related to actual PHY
>> +		 * routing or rather the hardware is just reusing the same bit,
>> +		 * don't check phy0_dual_route here.
>> +		 */
> Ditto
>
> Thanks!
> Maxime


-- 
С уважением,
     Евгений Богер / Evgeny Boger
     CTO, Wiren Board Team
     https://wirenboard.com/ru
     +7 495 150 66 19 (# 33)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 4128 bytes --]

Hi,

On Wed, Jan 12, 2022 at 11:59:31AM +0300, Evgeny Boger wrote:
> Hi Maxime!
> 12.01.2022 11:42, Maxime Ripard пишет:
> > Hi,
> > 
> > On Tue, Jan 11, 2022 at 07:51:53PM +0300, Evgeny Boger wrote:
> > > We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
> > > in our custom A40i-based design. What we observe is that after several
> > > attempts USB device would fail to enumerate on EHCI port (HS) and will be
> > > enumerated instead on OHCI (FS, 12Mb/s) only:
> > > 
> > > [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
> > > [    6.818008] usb 1-1: device not accepting address 5, error -71
> > > [    6.823868] usb usb1-port1: unable to enumerate USB device
> > > [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
> > > [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
> > > 
> > > On some boards one of the ports would work in high-speed mode, but
> > > on most of the boards all three USB ports would only work in FS mode.
> > > 
> > > At the same time, USB work flawlessly in high-speed mode in vendor kernel.
> > > 
> > > Looking for the differences in USB code, we found the issue with USB PHY
> > > register initialization. Basically, USB PHY driver sets a couple of
> > > internal undocumented PHY registers to the predefined constants. These PHY
> > > registers are accessed in a very (and I mean VERY) weird way by shifting
> > > register addresses and values bit-by-bit. This access method was slightly
> > > changed starting from H3 SoC, according to the BSP source for different
> > > SoCs.
> > > 
> > > As a result, mainline PHY driver won't set these PHY registers properly
> > > resulting in unreliable enumeration in high-speed mode.
> > > 
> > > We don't know whether this issue will result in broken HS mode on all
> > > affected SoCs or instead the A40i is an unfortunate exception. What we
> > > indeed verified, is that BSPs for all affected SoCs write these registers
> > > properly while mainline kernel don't. We also were able to reproduce the
> > > USB issue on a couple of A40i boards from other vendors, so we are pretty
> > > sure these registers have to always be properly set, regardlress of
> > > a hardware layout.
> > > 
> > > The proposed patch is tested on A40i-based Wiren Board 7 building
> > > automation controller. More details are below.
> > > 
> > > On older SoCs (prior to H3) PHY register are accessed by manipulating
> > > the common register for all PHYs. PHY index is specified by pulsing
> > > usbc bit.
> > > 
> > > Newer SoCs leave the access procedure mostly unchanged, the
> > > difference being that the latch registers are separate for each PHY.
> > > 
> > > Additionally, accessing USB PHY registers is only possible if phy0 is
> > > routed to musb IP instead of HCI.
> > > 
> > > Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
> > > R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
> > > 
> > > On A83t, H6, H616, T507 and probably on more recent hardware,
> > > these PHY registers are not used in vendor BSP.
> > > So don't set v2 flag for these even newer SoCs as a precaution.
> > > 
> > > Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> > This should probably be sent to stable, and have a Fixes tag?
> 
> well, phy register writes never worked on H3 SoC and newer, so there is no
> specific commit which broke it. So nothing to put to Fixes: tag.

If anything, it should be the patch that introduces the H3 support?

> I'm not sure it qualifies for stable too. Citing guidelines:
> 
> >It must be obviously correct and tested
> 
> I was only able to test it on handful of A40i boards and on one A20 board
> 
> >It must fix a real bug that bothers people
> 
> this bug for certain bothers *us*, but our users won't benefit from porting
> this fix to stable.
> I didn't find reports on this bug from other people.
> I think unfortunately not many people are using mainline kernel on these
> SoCs.

Yeah, it makes sense then, thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Samuel Holland]

On 1/11/22 10:51 AM, Evgeny Boger wrote:
> We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
> in our custom A40i-based design. What we observe is that after several
> attempts USB device would fail to enumerate on EHCI port (HS) and will be
> enumerated instead on OHCI (FS, 12Mb/s) only:
> 
> [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
> [    6.818008] usb 1-1: device not accepting address 5, error -71
> [    6.823868] usb usb1-port1: unable to enumerate USB device
> [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
> [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
> 
> On some boards one of the ports would work in high-speed mode, but
> on most of the boards all three USB ports would only work in FS mode.
> 
> At the same time, USB work flawlessly in high-speed mode in vendor kernel.
> 
> Looking for the differences in USB code, we found the issue with USB PHY
> register initialization. Basically, USB PHY driver sets a couple of
> internal undocumented PHY registers to the predefined constants. These PHY
> registers are accessed in a very (and I mean VERY) weird way by shifting
> register addresses and values bit-by-bit. This access method was slightly
> changed starting from H3 SoC, according to the BSP source for different
> SoCs.
> 
> As a result, mainline PHY driver won't set these PHY registers properly
> resulting in unreliable enumeration in high-speed mode.
> 
> We don't know whether this issue will result in broken HS mode on all
> affected SoCs or instead the A40i is an unfortunate exception. What we
> indeed verified, is that BSPs for all affected SoCs write these registers
> properly while mainline kernel don't. We also were able to reproduce the
> USB issue on a couple of A40i boards from other vendors, so we are pretty
> sure these registers have to always be properly set, regardlress of

typo: regardless

> a hardware layout.
> 
> The proposed patch is tested on A40i-based Wiren Board 7 building
> automation controller. More details are below.
> 
> On older SoCs (prior to H3) PHY register are accessed by manipulating
> the common register for all PHYs. PHY index is specified by pulsing
> usbc bit.
> 
> Newer SoCs leave the access procedure mostly unchanged, the
> difference being that the latch registers are separate for each PHY.
> 
> Additionally, accessing USB PHY registers is only possible if phy0 is
> routed to musb IP instead of HCI.
> 
> Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
> R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
> 
> On A83t, H6, H616, T507 and probably on more recent hardware,
> these PHY registers are not used in vendor BSP.
> So don't set v2 flag for these even newer SoCs as a precaution.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

Tested-by: Samuel Holland <samuel@sholland.org> # A40i, H3

I tested this patch on a Banana Pi M2 Berry (A40i) board. My board did not have
any USB reliability problems without this patch, but the patch didn't break
anything either. So since it fixes USB for you, the patch looks like a net
improvement.

And with Maxime's comments resolved:

Reviewed-by: Samuel Holland <samuel@sholland.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Icenowy Zheng]

于 2022年4月10日 GMT+08:00 下午12:28:25, Samuel Holland <samuel@sholland.org> 写到:
>On 1/11/22 10:51 AM, Evgeny Boger wrote:
>> We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
>> in our custom A40i-based design. What we observe is that after several
>> attempts USB device would fail to enumerate on EHCI port (HS) and will be
>> enumerated instead on OHCI (FS, 12Mb/s) only:
>> 
>> [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
>> [    6.818008] usb 1-1: device not accepting address 5, error -71
>> [    6.823868] usb usb1-port1: unable to enumerate USB device
>> [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
>> [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
>> 
>> On some boards one of the ports would work in high-speed mode, but
>> on most of the boards all three USB ports would only work in FS mode.
>> 
>> At the same time, USB work flawlessly in high-speed mode in vendor kernel.
>> 
>> Looking for the differences in USB code, we found the issue with USB PHY
>> register initialization. Basically, USB PHY driver sets a couple of
>> internal undocumented PHY registers to the predefined constants. These PHY
>> registers are accessed in a very (and I mean VERY) weird way by shifting
>> register addresses and values bit-by-bit. This access method was slightly
>> changed starting from H3 SoC, according to the BSP source for different
>> SoCs.
>> 
>> As a result, mainline PHY driver won't set these PHY registers properly
>> resulting in unreliable enumeration in high-speed mode.
>> 
>> We don't know whether this issue will result in broken HS mode on all
>> affected SoCs or instead the A40i is an unfortunate exception. What we
>> indeed verified, is that BSPs for all affected SoCs write these registers
>> properly while mainline kernel don't. We also were able to reproduce the
>> USB issue on a couple of A40i boards from other vendors, so we are pretty
>> sure these registers have to always be properly set, regardlress of
>
>typo: regardless
>
>> a hardware layout.
>> 
>> The proposed patch is tested on A40i-based Wiren Board 7 building
>> automation controller. More details are below.
>> 
>> On older SoCs (prior to H3) PHY register are accessed by manipulating
>> the common register for all PHYs. PHY index is specified by pulsing
>> usbc bit.
>> 
>> Newer SoCs leave the access procedure mostly unchanged, the
>> difference being that the latch registers are separate for each PHY.
>> 
>> Additionally, accessing USB PHY registers is only possible if phy0 is
>> routed to musb IP instead of HCI.
>> 
>> Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
>> R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
>> 
>> On A83t, H6, H616, T507 and probably on more recent hardware,
>> these PHY registers are not used in vendor BSP.
>> So don't set v2 flag for these even newer SoCs as a precaution.
>> 
>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
>
>Tested-by: Samuel Holland <samuel@sholland.org> # A40i, H3
>
>I tested this patch on a Banana Pi M2 Berry (A40i) board. My board did not have
>any USB reliability problems without this patch, but the patch didn't break
>anything either. So since it fixes USB for you, the patch looks like a net
>improvement.

From my memory, R40 OTG used to be broken.

I may check whether this is fixed by this patch when I am back home (I am
trapped in Shanghai because of COVID now).

>
>And with Maxime's comments resolved:
>
>Reviewed-by: Samuel Holland <samuel@sholland.org>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Evgeny Boger]

10.04.2022 07:36, Icenowy Zheng пишет:
>
> 于 2022年4月10日 GMT+08:00 下午12:28:25, Samuel Holland <samuel@sholland.org> 写到:
>> On 1/11/22 10:51 AM, Evgeny Boger wrote:
>>> We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
>>> in our custom A40i-based design. What we observe is that after several
>>> attempts USB device would fail to enumerate on EHCI port (HS) and will be
>>> enumerated instead on OHCI (FS, 12Mb/s) only:
>>>
>>> [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
>>> [    6.818008] usb 1-1: device not accepting address 5, error -71
>>> [    6.823868] usb usb1-port1: unable to enumerate USB device
>>> [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
>>> [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
>>>
>>> On some boards one of the ports would work in high-speed mode, but
>>> on most of the boards all three USB ports would only work in FS mode.
>>>
>>> At the same time, USB work flawlessly in high-speed mode in vendor kernel.
>>>
>>> Looking for the differences in USB code, we found the issue with USB PHY
>>> register initialization. Basically, USB PHY driver sets a couple of
>>> internal undocumented PHY registers to the predefined constants. These PHY
>>> registers are accessed in a very (and I mean VERY) weird way by shifting
>>> register addresses and values bit-by-bit. This access method was slightly
>>> changed starting from H3 SoC, according to the BSP source for different
>>> SoCs.
>>>
>>> As a result, mainline PHY driver won't set these PHY registers properly
>>> resulting in unreliable enumeration in high-speed mode.
>>>
>>> We don't know whether this issue will result in broken HS mode on all
>>> affected SoCs or instead the A40i is an unfortunate exception. What we
>>> indeed verified, is that BSPs for all affected SoCs write these registers
>>> properly while mainline kernel don't. We also were able to reproduce the
>>> USB issue on a couple of A40i boards from other vendors, so we are pretty
>>> sure these registers have to always be properly set, regardlress of
>> typo: regardless
>>
>>> a hardware layout.
>>>
>>> The proposed patch is tested on A40i-based Wiren Board 7 building
>>> automation controller. More details are below.
>>>
>>> On older SoCs (prior to H3) PHY register are accessed by manipulating
>>> the common register for all PHYs. PHY index is specified by pulsing
>>> usbc bit.
>>>
>>> Newer SoCs leave the access procedure mostly unchanged, the
>>> difference being that the latch registers are separate for each PHY.
>>>
>>> Additionally, accessing USB PHY registers is only possible if phy0 is
>>> routed to musb IP instead of HCI.
>>>
>>> Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
>>> R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
>>>
>>> On A83t, H6, H616, T507 and probably on more recent hardware,
>>> these PHY registers are not used in vendor BSP.
>>> So don't set v2 flag for these even newer SoCs as a precaution.
>>>
>>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
>> Tested-by: Samuel Holland <samuel@sholland.org> # A40i, H3
>>
>> I tested this patch on a Banana Pi M2 Berry (A40i) board. My board did not have
>> any USB reliability problems without this patch, but the patch didn't break
>> anything either. So since it fixes USB for you, the patch looks like a net
>> improvement.
>  From my memory, R40 OTG used to be broken.
>
> I may check whether this is fixed by this patch when I am back home (I am
> trapped in Shanghai because of COVID now).
Hi Icenowy,

I don't think OTG being broken has something to do with this patch.

Actually, on R40 USB OTG is implemented in the same way as on many other 
Allwinner SoCs: there are both HCI and OTG controllers for the first USB 
port and
there is a PHY which can route signals to either one.

Vendor kernel reroute signals to HCI controller whenever a host role is 
activated. The mainline kernel does the same, albeit in a very obscure way.

So enabling OTG on R40 is just a matter of adding a few lines to dtsi 
and, preferably, making a few modifications to phy code.

Could you please test OTG using our kernel tree: 
https://github.com/wirenboard/linux ?

There a few patches there waiting for being submitted which make it work.


>
>> And with Maxime's comments resolved:
>>
>> Reviewed-by: Samuel Holland <samuel@sholland.org>
>>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] phy: sun4i-usb: fix phy write on H3 and newer

[Evgeny Boger]

> On 1/11/22 10:51 AM, Evgeny Boger wrote:
>> We noticed that USB hosts won't reliably enumerate USB HS (480Mb/s) devices
>> in our custom A40i-based design. What we observe is that after several
>> attempts USB device would fail to enumerate on EHCI port (HS) and will be
>> enumerated instead on OHCI (FS, 12Mb/s) only:
>>
>> [    6.368009] usb 1-1: new high-speed USB device number 5 using ehci-platform
>> [    6.818008] usb 1-1: device not accepting address 5, error -71
>> [    6.823868] usb usb1-port1: unable to enumerate USB device
>> [    7.308013] usb 3-1: new full-speed USB device number 2 using ohci-platform
>> [    7.575045] usb 3-1: not running at top speed; connect to a high speed hub
>>
>> On some boards one of the ports would work in high-speed mode, but
>> on most of the boards all three USB ports would only work in FS mode.
>>
>> At the same time, USB work flawlessly in high-speed mode in vendor kernel.
>>
>> Looking for the differences in USB code, we found the issue with USB PHY
>> register initialization. Basically, USB PHY driver sets a couple of
>> internal undocumented PHY registers to the predefined constants. These PHY
>> registers are accessed in a very (and I mean VERY) weird way by shifting
>> register addresses and values bit-by-bit. This access method was slightly
>> changed starting from H3 SoC, according to the BSP source for different
>> SoCs.
>>
>> As a result, mainline PHY driver won't set these PHY registers properly
>> resulting in unreliable enumeration in high-speed mode.
>>
>> We don't know whether this issue will result in broken HS mode on all
>> affected SoCs or instead the A40i is an unfortunate exception. What we
>> indeed verified, is that BSPs for all affected SoCs write these registers
>> properly while mainline kernel don't. We also were able to reproduce the
>> USB issue on a couple of A40i boards from other vendors, so we are pretty
>> sure these registers have to always be properly set, regardlress of
> typo: regardless
>
>> a hardware layout.
>>
>> The proposed patch is tested on A40i-based Wiren Board 7 building
>> automation controller. More details are below.
>>
>> On older SoCs (prior to H3) PHY register are accessed by manipulating
>> the common register for all PHYs. PHY index is specified by pulsing
>> usbc bit.
>>
>> Newer SoCs leave the access procedure mostly unchanged, the
>> difference being that the latch registers are separate for each PHY.
>>
>> Additionally, accessing USB PHY registers is only possible if phy0 is
>> routed to musb IP instead of HCI.
>>
>> Introduce phy_reg_access_v2 cfg flag for H3 (H2+, H5),
>> R40 (V40, A40i, T3), V3s (V3, S3) and A64 SoCs.
>>
>> On A83t, H6, H616, T507 and probably on more recent hardware,
>> these PHY registers are not used in vendor BSP.
>> So don't set v2 flag for these even newer SoCs as a precaution.
>>
>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> Tested-by: Samuel Holland <samuel@sholland.org> # A40i, H3
>
> I tested this patch on a Banana Pi M2 Berry (A40i) board. My board did not have
> any USB reliability problems without this patch, but the patch didn't break
> anything either. So since it fixes USB for you, the patch looks like a net
> improvement.
Samuel, thank you for testing this!
>
> And with Maxime's comments resolved:
>
> Reviewed-by: Samuel Holland <samuel@sholland.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel