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

[Maxime Ripard <maxime@cerno.tech>]

[-- 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