Hi Brian,
On 2017/7/15 4:47, Brian Norris wrote:
> Hi Shawn,
>
> On Fri, Jul 14, 2017 at 11:52:46AM +0800, Shawn Lin wrote:
>> Deprecate legacy PHY model and encourage to use per-lane PHY
>> model.
>>
>> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>
>>
>> ---
>>
>> .../devicetree/bindings/pci/rockchip-pcie.txt | 25 ++++++++++++++++++++--
>> 1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> index 1453a73..78d4469 100644
>> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
>> @@ -19,8 +19,6 @@ Required properties:
>> - "pm"
>> - msi-map: Maps a Requester ID to an MSI controller and associated
>> msi-specifier data. See ./pci-msi.txt
>> -- phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
>> -- phy-names: MUST be "pcie-phy".
>> - interrupts: Three interrupt entries must be specified.
>> - interrupt-names: Must include the following names
>> - "sys"
>> @@ -42,6 +40,18 @@ Required properties:
>> interrupt source. The value must be 1.
>> - interrupt-map-mask and interrupt-map: standard PCI properties
>>
>> +Required properties for legacy PHY model (deprecated):
>> +- phys: From PHY bindings: Phandle for the Generic PHY for PCIe.
>> +- phy-names: MUST be "pcie-phy".
>> +
>> +Required properties for per-lane PHY model:
>> +- phys: Must contain an phandle to a PHY for each entry in phy-names.
>> +- phy-names: Must include an entry for each active lane. Note that the number
>> + of entries does not have to (though usually will) be equal to the specified
>> + number of lanes in the num-lanes property. Entries are of the form
>> + "pcie-phy-N": where N ranges from 0 to the value specified of (num-lanes - 1).
>> + (see example below)
>
> So (for the non-legacy case) are you requiring listing all 4 PHY lanes,
> or not?
I intended to do that however that makes the code more complex and I
didn't see any gains here. So I would say "list 4 of them here is
mandatory".
Will update this Doc to reflect the fact.
Thanks.
If you aren't, then you need to make the PHY driver handle the
> case were lanes {X..3} were never "powered on", but we want them idled.
> IIUC, your current (broken) implementation is trying (incorrectly) to
> only idle a lane once it has been powered on and then back off. But that
> won't ever happen if we only request (for example) PHY lane 0.
>
> It's also misleading that it's possible to specify only some subset of
> the PHY lanes, but the driver might still try to use them al
>
> Brian
>
>> +
>> Optional Property:
>> - aspm-no-l0s: RC won't support ASPM L0s. This property is needed if
>> using 24MHz OSC for RC's PHY.
>> @@ -95,6 +105,7 @@ pcie0: pcie at f8000000 {
>> <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
>> reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
>> "pm", "pclk", "aclk";
>> + /* deprecated legacy PHY model */
>> phys = <&pcie_phy>;
>> phy-names = "pcie-phy";
>> pinctrl-names = "default";
>> @@ -111,3 +122,13 @@ pcie0: pcie at f8000000 {
>> #interrupt-cells = <1>;
>> };
>> };
>> +
>> +pcie0: pcie at f8000000 {
>> + ...
>> +
>> + /* preferred per-lane PHY model */
>> + phys = <&pcie_phy 0>, <&pcie_phy 1>, <&pcie_phy 2>, <&pcie_phy 3>;
>> + phy-names = "pcie-phy-0", "pcie-phy-1", "pcie-phy-2", "pcie-phy-3";
>> +
>> + ...
>> +};
>> --
>> 1.9.1
>>
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>
