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@xxxxxxx>
> > Sent: Monday, October 21, 2019 7:46 PM
> > To: Anvesh Salveru <anvesh.s@xxxxxxxxxxx>
> > Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> > gustavo.pimentel@xxxxxxxxxxxx; jingoohan1@xxxxxxxxx; robh+dt@xxxxxxxxxx;
> > mark.rutland@xxxxxxx; Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> > 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@xxxxxxxxxxx>
> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> > > ---
> > >  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
> > >
>