RE: [PATCH 2/2] PCI: dwc: Add support to handle ZRX-DC Compliant PHYs

[Pankaj Dubey]

> -----Original Message-----
> From: Andrew Murray <andrew.murray@xxxxxxx>
> Sent: Monday, October 21, 2019 7:47 PM
> To: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> Cc: 'Anvesh Salveru' <anvesh.s@xxxxxxxxxxx>; linux-pci@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> gustavo.pimentel@xxxxxxxxxxxx; jingoohan1@xxxxxxxxx;
> lorenzo.pieralisi@xxxxxxx
> Subject: Re: [PATCH 2/2] PCI: dwc: Add support to handle ZRX-DC Compliant
> PHYs
> 
> On Mon, Oct 21, 2019 at 07:44:25PM +0530, Pankaj Dubey wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@xxxxxxx>
> > > Sent: Monday, October 21, 2019 7:34 PM
> > > To: Anvesh Salveru <anvesh.s@xxxxxxxxxxx>
> > > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > bhelgaas@xxxxxxxxxx; gustavo.pimentel@xxxxxxxxxxxx;
> > > jingoohan1@xxxxxxxxx; lorenzo.pieralisi@xxxxxxx; Pankaj Dubey
> > > <pankaj.dubey@xxxxxxxxxxx>
> > > Subject: Re: [PATCH 2/2] PCI: dwc: Add support to handle ZRX-DC
> > > Compliant PHYs
> > >
> > > On Mon, Oct 21, 2019 at 05:59:53PM +0530, Anvesh Salveru wrote:
> > > > Many platforms use DesignWare controller but the PHY can be
> > > > different in different platforms. If the PHY is compliant is to
> > > > ZRX-DC specification
> > >
> > > s/is to/to the/
> >
> > OK
> >
> > >
> > > > it helps in low power consumption during power states.
> > >
> > > s/in low/lower/
> > >
> > OK
> > > >
> > > > If current data rate is 8.0 GT/s or higher and PHY is not
> > > > compliant to ZRX-DC specification, then after every 100ms link
> > > > should transition to recovery state during the low power states.
> > > >
> > > > DesignWare controller provides GEN3_ZRXDC_NONCOMPL field in
> > > > GEN3_RELATED_OFF to specify about ZRX-DC compliant PHY.
> > > >
> > > > Platforms with ZRX-DC compliant PHY can set
"snps,phy-zrxdc-compliant"
> > > > property in controller DT node to specify this property to the
> > controller.
> > > >
> > > > Signed-off-by: Anvesh Salveru <anvesh.s@xxxxxxxxxxx>
> > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-designware.c | 7 +++++++
> > > > drivers/pci/controller/dwc/pcie-designware.h | 3 +++
> > > >  2 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 820488dfeaed..6560d9f765d7 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -556,4 +556,11 @@ void dw_pcie_setup(struct dw_pcie *pci)
> > > >  		       PCIE_PL_CHK_REG_CHK_REG_START;
> > > >  		dw_pcie_writel_dbi(pci,
PCIE_PL_CHK_REG_CONTROL_STATUS,
> > > val);
> > > >  	}
> > > > +
> > > > +	if (of_property_read_bool(np, "snps,phy-zrxdc-compliant")) {
> > > > +		val = dw_pcie_readl_dbi(pci,
PCIE_PORT_GEN3_RELATED);
> > > > +		val &= ~PORT_LOGIC_GEN3_ZRXDC_NONCOMPL;
> > > > +		dw_pcie_writel_dbi(pci, PCIE_PORT_GEN3_RELATED,
val);
> > > > +	}
> > > > +
> > >
> > > Given that this duplicates tegra_pcie_prepare_host in
> > > pcie-tegra194.c, can
> > we
> > > update that driver to adopt this new binding?
> > >
> >
> > Yes, Thanks for highlighting this. Otherwise I was worried that we
> > have one more patch without real user :)
> 
> Indeed :|
> 
> Though besides Tegra, is there any other reason you are adding this?
> 

Yes. We have one internal PCIe RC driver (which we can't disclose/upstream
right now) has this issue and currently we are handling it using this DT
binding. So we would like to upstream common code, so other platform's
driver can use this.

> > We will update pcie-tegra194.c driver and post the patch to adopt this
> > binding.
> 
> It's much appreciated.
> 
> Andrew Murray
> 
> >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > >  }
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index 5a18e94e52c8..427a55ec43c6 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -60,6 +60,9 @@
> > > >  #define PCIE_MSI_INTR0_MASK		0x82C
> > > >  #define PCIE_MSI_INTR0_STATUS		0x830
> > > >
> > > > +#define PCIE_PORT_GEN3_RELATED		0x890
> > > > +#define PORT_LOGIC_GEN3_ZRXDC_NONCOMPL		BIT(0)
> > > > +
> > > >  #define PCIE_ATU_VIEWPORT		0x900
> > > >  #define PCIE_ATU_REGION_INBOUND		BIT(31)
> > > >  #define PCIE_ATU_REGION_OUTBOUND	0
> > > > --
> > > > 2.17.1
> > > >
> >