[PATCH 1/2] dt-bindings: PCI: designware: Add binding for ZRX-DC PHY property

[PATCH 1/2] dt-bindings: PCI: designware: Add binding for ZRX-DC PHY property

[Anvesh Salveru]

Add support for ZRX-DC compliant PHYs. If PHY is not compliant to ZRX-DC
specification, then after every 100ms link should transition to recovery
state during the low power states which increases power consumption.

Platforms with ZRX-DC compliant PHY can use "snps,phy-zrxdc-compliant"
property in DesignWare controller DT node.

Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 Documentation/devicetree/bindings/pci/designware-pcie.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 78494c4050f7..9507ac38ac89 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -38,6 +38,8 @@ Optional properties:
    for data corruption. CDM registers include standard PCIe configuration
    space registers, Port Logic registers, DMA and iATU (internal Address
    Translation Unit) registers.
+- snps,phy-zrxdc-compliant: This property is needed if phy complies with the
+  ZRX-DC specification.
 RC mode:
 - num-viewport: number of view ports configured in hardware. If a platform
   does not specify it, the driver assumes 2.
-- 
2.17.1

Re: [PATCH 1/2] dt-bindings: PCI: designware: Add binding for ZRX-DC PHY property

[Andrew Murray]

On Mon, Oct 21, 2019 at 05:55:55PM +0530, Anvesh Salveru wrote:
> Add support for ZRX-DC compliant PHYs. If PHY is not compliant to ZRX-DC
> specification, then after every 100ms link should transition to recovery
> state during the low power states which increases power consumption.
> 
> Platforms with ZRX-DC compliant PHY can use "snps,phy-zrxdc-compliant"
> property in DesignWare controller DT node.
> 
> Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 78494c4050f7..9507ac38ac89 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -38,6 +38,8 @@ Optional properties:
>     for data corruption. CDM registers include standard PCIe configuration
>     space registers, Port Logic registers, DMA and iATU (internal Address
>     Translation Unit) registers.
> +- snps,phy-zrxdc-compliant: This property is needed if phy complies with the

Strictly speaking, this is a property of the phy - not the controller that
uses it.

If I understand correctly, there are some DW based PCI controllers that use
a phandle reference in DT to a Phy (such as fsl,imx6q-pcie.txt). Therefore
it feels like this is in the wrong place. Is there a reason this isn't
described in the Phy?

Thanks,

Andrew Murray

> +  ZRX-DC specification.
>  RC mode:
>  - num-viewport: number of view ports configured in hardware. If a platform
>    does not specify it, the driver assumes 2.
> -- 
> 2.17.1
> 

RE: [PATCH 1/2] dt-bindings: PCI: designware: Add binding for ZRX-DC PHY property

[Pankaj Dubey]

> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: Monday, October 21, 2019 7:46 PM
> To: Anvesh Salveru <anvesh.s@samsung.com>
> Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; bhelgaas@google.com;
> gustavo.pimentel@synopsys.com; jingoohan1@gmail.com; robh+dt@kernel.org;
> mark.rutland@arm.com; Pankaj Dubey <pankaj.dubey@samsung.com>
> Subject: Re: [PATCH 1/2] dt-bindings: PCI: designware: Add binding for
ZRX-DC
> PHY property
> 
> On Mon, Oct 21, 2019 at 05:55:55PM +0530, Anvesh Salveru wrote:
> > Add support for ZRX-DC compliant PHYs. If PHY is not compliant to
> > ZRX-DC specification, then after every 100ms link should transition to
> > recovery state during the low power states which increases power
> consumption.
> >
> > Platforms with ZRX-DC compliant PHY can use "snps,phy-zrxdc-compliant"
> > property in DesignWare controller DT node.
> >
> > Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/pci/designware-pcie.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > index 78494c4050f7..9507ac38ac89 100644
> > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > @@ -38,6 +38,8 @@ Optional properties:
> >     for data corruption. CDM registers include standard PCIe
configuration
> >     space registers, Port Logic registers, DMA and iATU (internal
Address
> >     Translation Unit) registers.
> > +- snps,phy-zrxdc-compliant: This property is needed if phy complies
> > +with the
> 
> Strictly speaking, this is a property of the phy - not the controller that
uses it.
> 
> If I understand correctly, there are some DW based PCI controllers that
use a
> phandle reference in DT to a Phy (such as fsl,imx6q-pcie.txt). Therefore
it feels
> like this is in the wrong place. Is there a reason this isn't described in
the Phy?
>

Yes, from HW point of view this is a property of the PHY. As PHY is the one
which is ZRXDC compliant or non-compliant. 
But as the DW controller programming needs to be altered for handling such
phys, so we added it as a DT binding of DW controller driver. 
Also it might be possible that, some other PCIe controller (other than
DesignWare), do not have any such provision in controller H/W and they
expect PHY itself should expose some SFR to handle such scenario. In such
cases it is straight-forward to add this binding as part of PHY node.

We can add this as part of PHY binding, but in that case we will end up
checking PHY binding in DWC driver via PHY nodes which seems little a bit of
hack. 

Do you have any other better approach to handle this? 
 
> Thanks,
> 
> Andrew Murray
> 
> > +  ZRX-DC specification.
> >  RC mode:
> >  - num-viewport: number of view ports configured in hardware. If a
platform
> >    does not specify it, the driver assumes 2.
> > --
> > 2.17.1
> >

Re: [PATCH 1/2] dt-bindings: PCI: designware: Add binding for ZRX-DC PHY property

[Andrew Murray]

On Mon, Oct 21, 2019 at 08:26:28PM +0530, Pankaj Dubey wrote:
> 
> 
> > -----Original Message-----
> > From: Andrew Murray <andrew.murray@arm.com>
> > Sent: Monday, October 21, 2019 7:46 PM
> > To: Anvesh Salveru <anvesh.s@samsung.com>
> > Cc: linux-pci@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; bhelgaas@google.com;
> > gustavo.pimentel@synopsys.com; jingoohan1@gmail.com; robh+dt@kernel.org;
> > mark.rutland@arm.com; Pankaj Dubey <pankaj.dubey@samsung.com>
> > Subject: Re: [PATCH 1/2] dt-bindings: PCI: designware: Add binding for
> ZRX-DC
> > PHY property
> > 
> > On Mon, Oct 21, 2019 at 05:55:55PM +0530, Anvesh Salveru wrote:
> > > Add support for ZRX-DC compliant PHYs. If PHY is not compliant to
> > > ZRX-DC specification, then after every 100ms link should transition to
> > > recovery state during the low power states which increases power
> > consumption.
> > >
> > > Platforms with ZRX-DC compliant PHY can use "snps,phy-zrxdc-compliant"
> > > property in DesignWare controller DT node.
> > >
> > > Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/designware-pcie.txt | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > index 78494c4050f7..9507ac38ac89 100644
> > > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > @@ -38,6 +38,8 @@ Optional properties:
> > >     for data corruption. CDM registers include standard PCIe
> configuration
> > >     space registers, Port Logic registers, DMA and iATU (internal
> Address
> > >     Translation Unit) registers.
> > > +- snps,phy-zrxdc-compliant: This property is needed if phy complies
> > > +with the
> > 
> > Strictly speaking, this is a property of the phy - not the controller that
> uses it.
> > 
> > If I understand correctly, there are some DW based PCI controllers that
> use a
> > phandle reference in DT to a Phy (such as fsl,imx6q-pcie.txt). Therefore
> it feels
> > like this is in the wrong place. Is there a reason this isn't described in
> the Phy?
> >
> 
> Yes, from HW point of view this is a property of the PHY. As PHY is the one
> which is ZRXDC compliant or non-compliant. 
> But as the DW controller programming needs to be altered for handling such
> phys, so we added it as a DT binding of DW controller driver. 
> Also it might be possible that, some other PCIe controller (other than
> DesignWare), do not have any such provision in controller H/W and they
> expect PHY itself should expose some SFR to handle such scenario. In such
> cases it is straight-forward to add this binding as part of PHY node.
> 
> We can add this as part of PHY binding, but in that case we will end up
> checking PHY binding in DWC driver via PHY nodes which seems little a bit of
> hack. 
> 
> Do you have any other better approach to handle this? 

I think there may be others that have more informed opinions, such as
Kishon and Rob (on CC).

Thanks,

Andrew Murray

>  
> > Thanks,
> > 
> > Andrew Murray
> > 
> > > +  ZRX-DC specification.
> > >  RC mode:
> > >  - num-viewport: number of view ports configured in hardware. If a
> platform
> > >    does not specify it, the driver assumes 2.
> > > --
> > > 2.17.1
> > >
> 

RE: [PATCH 1/2] dt-bindings: PCI: designware: Add binding for ZRX-DC PHY property

[Gustavo Pimentel]

On Mon, Oct 21, 2019 at 13:25:55, Anvesh Salveru <anvesh.s@samsung.com> 
wrote:

> Add support for ZRX-DC compliant PHYs. If PHY is not compliant to ZRX-DC
> specification, then after every 100ms link should transition to recovery
> state during the low power states which increases power consumption.
> 
> Platforms with ZRX-DC compliant PHY can use "snps,phy-zrxdc-compliant"
> property in DesignWare controller DT node.
> 
> Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 78494c4050f7..9507ac38ac89 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -38,6 +38,8 @@ Optional properties:
>     for data corruption. CDM registers include standard PCIe configuration
>     space registers, Port Logic registers, DMA and iATU (internal Address
>     Translation Unit) registers.
> +- snps,phy-zrxdc-compliant: This property is needed if phy complies with the
> +  ZRX-DC specification.
>  RC mode:
>  - num-viewport: number of view ports configured in hardware. If a platform
>    does not specify it, the driver assumes 2.
> -- 
> 2.17.1


Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>