Re: [PATCH V8 3/4] PCI: tegra: Broadcast PME_Turn_Off message before link goes to L2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 27, 2018 at 12:14:12PM +0530, Manikanta Maddireddy wrote:
> 
> 
> On 26-Feb-18 9:24 PM, Lorenzo Pieralisi wrote:
> > On Tue, Feb 13, 2018 at 08:54:09PM +0530, Manikanta Maddireddy wrote:
> >> Per PCIe r3.0, sec 5.3.3.2.1, PCIe root port shoould broadcast PME_Turn_Off
> > 
> > s/shoould/should
> > 
> >> message before PCIe link goes to L2. PME_Turn_Off broadcast mechanism is
> >> implemented in AFI module. Each Tegra PCIe root port has its own
> >> PME_Turn_Off and PME_TO_Ack bitmap in AFI_PME register, program this
> >> register to broadcast PME_Turn_Off message.
> >>
> >> Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> >> Acked-by: Thierry Reding <treding@xxxxxxxxxx>
> >> Tested-by: Thierry Reding <treding@xxxxxxxxxx>
> >> ---
> >> V2:
> >> * no change in this patch
> >> V3:
> >> * add PME bitmap in soc data instead of using compatible string
> >> * replace while loop with readl_poll_timeout() for polling
> >> * commit log correction
> >> V4:
> >> * no change in this patch
> >> V5:
> >> * Rebased on linux-next
> >> V6:
> >> * no change in this patch
> >> V7:
> >> * Per port soc data is added for pme bits
> >> * list_for_each_entry_safe is changed to list_for_each_entry for pme turnoff
> >> V8:
> >> * Rebased on top of latest patch-2
> >>
> >>  drivers/pci/host/pci-tegra.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 66 insertions(+)
> >>
> >> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> >> index 17ccb5e3ba04..60b1d5e1cfa4 100644
> >> --- a/drivers/pci/host/pci-tegra.c
> >> +++ b/drivers/pci/host/pci-tegra.c
> >> @@ -31,6 +31,7 @@
> >>  #include <linux/delay.h>
> >>  #include <linux/export.h>
> >>  #include <linux/interrupt.h>
> >> +#include <linux/iopoll.h>
> >>  #include <linux/irq.h>
> >>  #include <linux/irqdomain.h>
> >>  #include <linux/kernel.h>
> >> @@ -153,6 +154,8 @@
> >>  #define  AFI_INTR_EN_FPCI_TIMEOUT	(1 << 7)
> >>  #define  AFI_INTR_EN_PRSNT_SENSE	(1 << 8)
> >>  
> >> +#define AFI_PCIE_PME		0xf0
> >> +
> >>  #define AFI_PCIE_CONFIG					0x0f8
> >>  #define  AFI_PCIE_CONFIG_PCIE_DISABLE(x)		(1 << ((x) + 1))
> >>  #define  AFI_PCIE_CONFIG_PCIE_DISABLE_ALL		0xe
> >> @@ -233,6 +236,8 @@
> >>  #define PADS_REFCLK_CFG_PREDI_SHIFT		8  /* 11:8 */
> >>  #define PADS_REFCLK_CFG_DRVI_SHIFT		12 /* 15:12 */
> >>  
> >> +#define PME_ACK_TIMEOUT 10000
> >> +
> >>  struct tegra_msi {
> >>  	struct msi_controller chip;
> >>  	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
> >> @@ -244,8 +249,16 @@ struct tegra_msi {
> >>  };
> >>  
> >>  /* used to differentiate between Tegra SoC generations */
> >> +struct tegra_pcie_port_soc {
> >> +	struct {
> >> +		u8 turnoff_bit;
> >> +		u8 ack_bit;
> >> +	} pme;
> >> +};
> >> +
> >>  struct tegra_pcie_soc {
> >>  	unsigned int num_ports;
> >> +	const struct tegra_pcie_port_soc *ports;
> >>  	unsigned int msi_base_shift;
> >>  	u32 pads_pll_ctl;
> >>  	u32 tx_ref_sel;
> >> @@ -1358,6 +1371,32 @@ static int tegra_pcie_put_resources(struct tegra_pcie *pcie)
> >>  	return 0;
> >>  }
> >>  
> >> +static void tegra_pcie_pme_turnoff(struct tegra_pcie_port *port)
> >> +{
> >> +	struct tegra_pcie *pcie = port->pcie;
> >> +	const struct tegra_pcie_soc *soc = pcie->soc;
> >> +	int err;
> >> +	u32 val;
> >> +	u8 ack_bit;
> >> +
> >> +	val = afi_readl(pcie, AFI_PCIE_PME);
> >> +	val |= (0x1 << soc->ports[port->index].pme.turnoff_bit);
> >> +	afi_writel(pcie, val, AFI_PCIE_PME);
> >> +
> >> +	ack_bit = soc->ports[port->index].pme.ack_bit;
> >> +	err = readl_poll_timeout(pcie->afi + AFI_PCIE_PME, val,
> >> +				 val & (0x1 << ack_bit), 1, PME_ACK_TIMEOUT);
> >> +	if (err)
> >> +		dev_err(pcie->dev, "PME Ack is not received on port: %d\n",
> >> +			port->index);
> >> +
> >> +	usleep_range(10000, 11000);
> >> +
> >> +	val = afi_readl(pcie, AFI_PCIE_PME);
> >> +	val &= ~(0x1 << soc->ports[port->index].pme.turnoff_bit);
> >> +	afi_writel(pcie, val, AFI_PCIE_PME);
> >> +}
> >> +
> >>  static int tegra_msi_alloc(struct tegra_msi *chip)
> >>  {
> >>  	int msi;
> >> @@ -2103,8 +2142,14 @@ static void tegra_pcie_disable_ports(struct tegra_pcie *pcie)
> >>  	}
> >>  }
> >>  
> >> +static const struct tegra_pcie_port_soc tegra20_pcie_ports[] = {
> >> +	{ .pme.turnoff_bit = 0, .pme.ack_bit =  5 },
> >> +	{ .pme.turnoff_bit = 8, .pme.ack_bit = 10 },
> >> +};
> >> +
> >>  static const struct tegra_pcie_soc tegra20_pcie = {
> >>  	.num_ports = 2,
> >> +	.ports = tegra20_pcie_ports,
> >>  	.msi_base_shift = 0,
> >>  	.pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> >>  	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> >> @@ -2118,8 +2163,15 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> >>  	.program_uphy = true,
> >>  };
> >>  
> >> +static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = {
> >> +	{ .pme.turnoff_bit =  0, .pme.ack_bit =  5 },
> >> +	{ .pme.turnoff_bit =  8, .pme.ack_bit = 10 },
> >> +	{ .pme.turnoff_bit = 16, .pme.ack_bit = 18 },
> >> +};
> >> +
> >>  static const struct tegra_pcie_soc tegra30_pcie = {
> >>  	.num_ports = 3,
> >> +	.ports = tegra30_pcie_ports,
> >>  	.msi_base_shift = 8,
> >>  	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> >>  	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> >> @@ -2136,6 +2188,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
> >>  
> >>  static const struct tegra_pcie_soc tegra124_pcie = {
> >>  	.num_ports = 2,
> >> +	.ports = tegra20_pcie_ports,
> >>  	.msi_base_shift = 8,
> >>  	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> >>  	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> >> @@ -2151,6 +2204,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
> >>  
> >>  static const struct tegra_pcie_soc tegra210_pcie = {
> >>  	.num_ports = 2,
> >> +	.ports = tegra20_pcie_ports,
> >>  	.msi_base_shift = 8,
> >>  	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> >>  	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> >> @@ -2164,8 +2218,15 @@ static const struct tegra_pcie_soc tegra210_pcie = {
> >>  	.program_uphy = true,
> >>  };
> >>  
> >> +static const struct tegra_pcie_port_soc tegra186_pcie_ports[] = {
> >> +	{ .pme.turnoff_bit =  0, .pme.ack_bit =  5 },
> >> +	{ .pme.turnoff_bit =  8, .pme.ack_bit = 10 },
> >> +	{ .pme.turnoff_bit = 12, .pme.ack_bit = 14 },
> >> +};
> >> +
> >>  static const struct tegra_pcie_soc tegra186_pcie = {
> >>  	.num_ports = 3,
> >> +	.ports = tegra186_pcie_ports,
> >>  	.msi_base_shift = 8,
> >>  	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> >>  	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> >> @@ -2399,12 +2460,17 @@ static int tegra_pcie_remove(struct platform_device *pdev)
> >>  {
> >>  	struct tegra_pcie *pcie = platform_get_drvdata(pdev);
> >>  	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> >> +	struct tegra_pcie_port *port;
> >>  
> >>  	if (IS_ENABLED(CONFIG_DEBUG_FS))
> >>  		tegra_pcie_debugfs_exit(pcie);
> >>  
> >>  	pci_stop_root_bus(host->bus);
> >>  	pci_remove_root_bus(host->bus);
> >> +
> >> +	list_for_each_entry(port, &pcie->ports, list)
> >> +		tegra_pcie_pme_turnoff(port);
> >> +
> > 
> > IIUC how it works this patch and the previous one should be swapped,
> > otherwise the driver removal may not work correctly.
> > 
> > It is just for my own understanding (and bisection reasons).
> > 
> > Thanks,
> > Lorenzo
> > 
> Hi Lorenzo,
> 
> Previous patch implements the driver removal code, without that
> we cannot add PME_TurnOff logic. IMO we need squash previous
> patch & this one to have working removal code with single patch.
> This takes care of bisection issue as well.

Yes that makes sense (inclusive of a merged commit log that explains
the HW requirements to ensure a working removal code).

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux