Re: [PATCH 02/14] dt-bindings: phy: qcom,qmp-usb3-dp: fix sc8280xp bindings

[Johan Hovold <johan@kernel.org>]

On Sat, Nov 12, 2022 at 02:43:03PM +0300, Dmitry Baryshkov wrote:
> On 11/11/2022 12:24, Johan Hovold wrote:
> > The current QMP USB3-DP PHY bindings are based on the original MSM8996
> > binding which provided multiple PHYs per IP block and these in turn were
> > described by child nodes.
> > 
> > The QMP USB3-DP PHY block provides a single multi-protocol PHY and even
> > if some resources are only used by either the USB or DP part of the
> > device there is no real benefit in describing these resources in child
> > nodes.
> > 
> > The original MSM8996 binding also ended up describing the individual
> > register blocks as belonging to either the wrapper node or the PHY child
> > nodes.
> > 
> > This is an unnecessary level of detail which has lead to problems when
> > later IP blocks using different register layouts have been forced to fit
> > the original mould rather than updating the binding. The bindings are
> > arguable also incomplete as they only the describe register blocks used
> > by the current Linux drivers (e.g. does not include the PCS LANE
> > registers).
> > 
> > This is specifically true for later USB4-USB3-DP QMP PHYs where the TX
> > registers are used by both the USB3 and DP parts of the PHY (and where
> > the USB4 part of the PHY was not covered by the binding at all). Notably
> > there are also no DP "RX" (sic) registers as described by the current
> > bindings and the DP "PCS" region is really a set of DP_PHY registers.
> > 
> > Add a new binding for the USB4-USB3-DP QMP PHYs found on SC8280XP which
> > further bindings can be based on.
> > 
> > Note that the binding uses a PHY type index to access either the USB3 or
> > DP part of the PHY and that this can later be used also for the USB4
> > part if needed.
> > 
> > Similarly, the clock inputs and outputs can later be extended to support
> > USB4.
> > 
> > Also note that the current binding is simply removed instead of being
> > deprecated as it was only recently merged and would not allow for
> > supporting DP mode.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---

> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  clock-output-names:
> > +    items:
> > +      - const: usb3_pipe
> > +      - const: dp_link
> > +      - const: dp_vco_div
> > +
> > +  "#phy-cells":
> > +    const: 1
> > +    description: |
> > +      PHY index
> > +        - PHY_TYPE_USB3
> > +        - PHY_TYPE_DP
> 
> I'm stepping on Rob's and Krzysztof's ground here, but it might be more 
> logical and future proof to use indices instead of phy types.

Why would that be more future-proof?

I initially added defines for these indexes to a QMP header, but noticed
that we already have PHY drivers that use the PHY types for this. So
there's already a precedent for this and I didn't see any real benefit
to adding multiple per-SoC defines for the same thing.

> Just for my understanding, would USB4 support add another qserdes+tx/rx 
> construct or would it be the same USB3 register space?

The TX/RX registers are shared by the all three parts of the PHY (USB4,
USB3, DP), while USB4 has two dedicated sets of PLL (serdes) and PCS
registers.

Johan