Re: [PATCH v3 6/6] phy: qcom-qusb2: Add QUSB2 PHYs support for sdm845

[Manu Gautam]

Hi,


On 3/28/2018 4:22 AM, Doug Anderson wrote:
> Hi,
>
> On Thu, Mar 22, 2018 at 11:11 PM, Manu Gautam <mgautam@xxxxxxxxxxxxxx> wrote:
>> There are two QUSB2 PHYs present on sdm845. Update PHY
>> registers programming for both the PHYs related to
>> electrical parameters to improve eye diagram.
> This tuning difference is truly associated with the SoC itself?  Are
> you sure?  Are the two different PHYs in the SoC somehow using
> different silicon processes?  ...or is one close to another IP block
> that is noisy?  ...or something else to account for this difference?
>
> It seems more likely that this tuning difference is associated with
> the board.  If you're _certain_ this is really due to internal SoC
> differences you'll have to come up with some darn good evidence to
> convince me...

This difference must be due to board only.

>
> If the tuning is truly associated with the board then:
>
> 1. You should have a single device tree compatible string.  IMHO it
> should contain the name of the SoC in it, so "qcom,sdm845-qusb2-phy".
> It's generally OK to name something in Linux using the name of the
> first thing that happened to support it in Linux (even if later
> processors use the exact same component).  Leaving it as just
> "qcom,qusb2-v2-phy" is OK with me too if that's what everyone wants.
I will remove "qcom,qusb2-v2-phy" as I don't expect any users of that.
>
>
> 2. You should figure out how to describe the needed board-to-board
> tuning in device tree.
>
> The only two differences you have right now are:
>
> QUSB2PHY_IMP_CTRL1: 0 => 0x8
> QUSB2PHY_PORT_TUNE1: 0x30 => 0x48
>
> I'm not sure I found all the correct documentation for the PHY (the
> docs I have say that "TUNE1" bit 3 is "reserved") so I can't come up
> with all of these for you.  But I think I found the difference
> accounting for the upper nybble of TUNE1 changing from 0x3 to 0x4.
> For this, I think you'd want a device tree property like:
>
> qcom,hstx_trim_mv
>
> ...and the values of that property would be the values from 800 to 950
> in 8 steps, or [800, 821, 842, 864, 885, 907, 928, 950].
>
> You'd want to do similar things for the other differences.
>
> You don't need to encode every possible difference right now.  When
> you come up with something that needs to be different you add a new
> optional device tree property (defaulting to whatever the driver used
> to do) to describe your new property.

Sure. I will come up with separate device tree properties to specify
board-to-board differences in PHY tuning.


>
> -Doug

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project