Date:   Mon, 14 Nov 2022 18:31:02 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Johan Hovold <johan@...nel.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Vinod Koul <vkoul@...nel.org>, Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/14] dt-bindings: phy: qcom,qmp-usb3-dp: fix sc8280xp
 bindings

On 14/11/2022 16:37, Johan Hovold wrote:
> 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@...nel.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.

As you guessed from my question, I was thinking about USB4 (for which we 
do not have a separate PHY_TYPE, but that probably shouldn't matter). 
Would it be a separate PHY here, or would it be a combo UBS3+USB4 plus 
separate DP phy?

Yes, we have other PHYs, which use types as an index, however it's 
slightly more common to have indices instead.

Anyway, this is a minor issue. It might be just that I'm more common to 
using indices everywhere (in other words, I have preference here, but 
it's not a strong requirement from my side).


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

Ack, thanks.

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists