Re: [PATCH 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,vendor-init-seq

[me@herrie.org]

On 2026-06-12 02:25, Dmitry Baryshkov wrote:
> On Thu, Jun 11, 2026 at 12:39:45PM +0200, Konrad Dybcio wrote:
>> On 6/4/26 1:02 AM, Dmitry Baryshkov wrote:
>> > On Wed, Jun 03, 2026 at 06:09:18PM +0200, me@herrie.org wrote:
>> >> On 2026-06-03 15:57, Dmitry Baryshkov wrote:
>> >>> On Wed, Jun 03, 2026 at 07:48:08AM +0200, Herman van Hazendonk wrote:
>> >>>> Add an optional "qcom,vendor-init-seq" property carrying raw ULPI
>> >>>> (address, value) pairs that are written after PHY reset.
>> >>>>
>> >>>> Unlike the existing "qcom,init-seq" property, the address field is
>> >>>> NOT offset by ULPI_EXT_VENDOR_SPECIFIC, so the new property can
>> >>>> reach the standard ULPI vendor register range (0x30-0x3f). MSM8x60-
>> >>>> class hardware needs this range to programme pre-emphasis, HS driver
>> >>>> slope and CDR auto-reset bits the legacy msm_otg driver used to set
>> >>>> via platform data.
>> >>>
>> >>> Are those register writes specific to the device or to the whole
>> >>> platform? In the latter case please extend the driver to write them.
>> >>
>> >> Looking at every MSM8x60 reference kernel I could find (Qualcomm's own
>> >> msm8x60 board, HP TouchPad / APQ8060, and some HTC/Saumsung MSM8660
>> >> devices), the writes split into two groups:
>> >>
>> >> Platform-level (same across all MSM8x60 hardware):
>> >>  - reg 0x36 bits 1+2: CDR auto-reset disabled, SE1 gating disabled
>> >>  - reg 0x32 bits [5:4]: pre-emphasis at 20%
>> >>
>> >> Board-specific:
>> >>  - reg 0x32 bits [3:0]: HS driver slope — HP TouchPad uses 5, HTC
>> >>    devices use 1. This clearly depends on board layout (trace length,
>> >>    connector loading, etc.).
>> >>
>> >> So the platform-level writes should move unconditionally into the driver
>> >> behind a match-data flag for the MSM8x60-class compatible, and only the
>> >> HS driver slope value belongs in DT.
>> >
>> > Looks like it. Please hardcode the value for your platform in the driver
>> > (with the comment), meanwhile we can try looking up the actual values.
>> 
>> Do we have the values for a MTP/QRD (or whatever they used to be 
>> called
>> back then..), like we would usually put in there?
> 
> As far as I can understand msm-3.0 and msm-3.4 most of the boards were
> writing 0 here (although it might have been unexpected). None of the
> board files set the hsdrvslope value (which means 0).
> 
> Please correct me if I'm wrong. I see that for tenderloin kernels 
> change
> that to 0x5, but I can't find a sensible commit message.
> 
> I could not find the documentation for vendor ULPI registers for those
> chips, so I don't think we can identify, how to make sense of those
> values. In such a case and having different board-specific values, we
> don't have a better option than having a qcom,hsdrvslope (or similarly
> named) property in DT.
Dmitry/Konrad,

The bit-level meaning *is* documented byte-for-byte across
every MSM8x60 downstream tree I could find: Code Aurora is the
canonical source:

     arch/arm/mach-msm/include/mach/msm_hsusb_hw.h

     #define ULPI_CONFIG_REG3        0x32
     #define ULPI_DIGOUT_CTRL        0x36          /* on MSM7x30 / 
MSM8x60 */
     #define ULPI_CDR_AUTORESET      (1 << 1)
     #define ULPI_SE1_GATE           (1 << 2)
     #define ULPI_PRE_EMPHASIS_MASK  (3 << 4)
     #define ULPI_HSDRVSLOPE_MASK    (0x0F)

   reg 0x32 [5:4]: pre-emphasis level (00 disabled, 11 = 20%, 10 = 10%)
   reg 0x32 [3:0]: HS driver slope
   reg 0x36 bit 1: CDR auto-reset enable
   reg 0x36 bit 2: SE1 gating enable

The legacy `msm72k_otg.c` semantics for the 0x36 bits are inverted:
setting the bit disables the function (see set_cdr_auto_reset() and
set_se1_gating()).  Every MSM8x60 reference board sets both bits,
i.e. CDR auto-reset disabled + SE1 gating disabled.

Surveying every MSM8x60-class board file I can reach: Qualcomm's own
reference, Samsung, Sony, HTC, and HP: the platform-level values
are unanimous:

   pre-emphasis     = 20%
   CDR auto-reset   = disabled
   SE1 gating       = disabled

|  Vendor / board                                 | hsdrvslope |
|  -----------------------------------------------|------------|
|  Qualcomm reference (SURF / FFA / Fluid /       |            |
|    Dragon / Fusion -- board-msm8x60.c)          | unset (0)  |
|  Samsung Galaxy S2 family (Q1 / Celox / Dali /  |            |
|    generic 8x60 MTP)                            | unset (0)  |
|  Sony MSM8660 (sony-kernel-msm8660)             | unset (0)  |
|  HTC MSM8660 (shooter / holiday / pyramid /     |            |
|    doubleshot / shooter_u / ruby)               | 1          |
|  HP TouchPad (board-tenderloin.c)               | 5          |

So Qualcomm's own MTP, the closest thing to a reference platform we
have, leaves hsdrvslope at the silicon default (0).  Two OEMs (HTC,
HP) override it to non-zero values that match their board layout.
Neither override has a commit message. HTC's value is buried in an
init_seq array, HP's appears as `.hsdrvslope = 0x05` in
board-tenderloin.c with no comment.  I've kept TouchPad's 5 because
that's the value the device shipped with and our HS link is happy
with it; changing it risks regressing eye margin on the silicon we
still have to test against.

So the plan is exactly what you both already converged on:

   1. Drop qcom,vendor-init-seq from the binding (this patch goes away
      entirely).
   2. In the driver, hardcode the three platform-level writes behind
      the qcom,usb-hs-phy-msm8660 compatible match-data, with a
      comment citing msm_hsusb_hw.h as the source for the bit names.
   3. Add a single qcom,hs-drv-slope property (u8, 0..15) for the
      board-varying value.  TouchPad DT sets 5.  Absent ⇒ leave silicon
      default in place, matching Qualcomm/Samsung/Sony reference
      behaviour.

I'll send the new v in that shape: single dt-bindings patch
(qcom,hs-drv-slope) plus the driver patch that hardcodes the platform
writes and consumes the new property.

Unless you have other suggestions?

Thanks,
Herman