Hi Bjorn,
This patch series does some modifications to powerup logic to avoid unbalanced
PM warnings, and turns off host power if no link is detected.
I also make some properties(eg, PHY/reset/..) optional to gain more flexibility
just in case future SoCs don't have it.
Could you also fold this patch series into pci/host-mediatek for v4.13?
Thanks!
Ryder Lee (3):
PCI: mediatek: modify controller powerup logic
PCI: mediatek: turn off subsys power if no link up detected
PCI: mediatek: make some properties optioanl
drivers/pci/host/pcie-mediatek.c | 110 +++++++++++++++++++++------------------
1 file changed, 58 insertions(+), 52 deletions(-)
--
1.9.1
There is no need to keep controller power on if slots are all empty,
so a check is added to turn it off. Besides, host controllers may not
need PM domain in some SoCs, thus we add dev->pm_domain to check that.
Signed-off-by: Ryder Lee <[email protected]>
---
drivers/pci/host/pcie-mediatek.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 165d82d..d256263 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -118,6 +118,18 @@ static inline bool mtk_pcie_link_is_up(struct mtk_pcie_port *port)
return !!(readl(port->base + PCIE_LINK_STATUS) & PCIE_PORT_LINKUP);
}
+static void mtk_pcie_subsys_powerdown(struct mtk_pcie *pcie)
+{
+ struct device *dev = pcie->dev;
+
+ clk_disable_unprepare(pcie->free_ck);
+
+ if (dev->pm_domain) {
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ }
+}
+
static void mtk_pcie_port_free(struct mtk_pcie_port *port)
{
struct mtk_pcie *pcie = port->pcie;
@@ -130,7 +142,6 @@ static void mtk_pcie_port_free(struct mtk_pcie_port *port)
static void mtk_pcie_put_resources(struct mtk_pcie *pcie)
{
- struct device *dev = pcie->dev;
struct mtk_pcie_port *port, *tmp;
list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
@@ -139,9 +150,7 @@ static void mtk_pcie_put_resources(struct mtk_pcie *pcie)
mtk_pcie_port_free(port);
}
- clk_disable_unprepare(pcie->free_ck);
- pm_runtime_put_sync(dev);
- pm_runtime_disable(dev);
+ mtk_pcie_subsys_powerdown(pcie);
}
static void __iomem *mtk_pcie_map_bus(struct pci_bus *bus,
@@ -340,10 +349,10 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
return PTR_ERR(pcie->free_ck);
}
- pm_runtime_enable(dev);
- err = pm_runtime_get_sync(dev);
- if (err)
- goto err_pm;
+ if (dev->pm_domain) {
+ pm_runtime_enable(dev);
+ pm_runtime_get_sync(dev);
+ }
/* enable top level clock */
err = clk_prepare_enable(pcie->free_ck);
@@ -355,9 +364,10 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
return 0;
err_free_ck:
- pm_runtime_put_sync(dev);
-err_pm:
- pm_runtime_disable(dev);
+ if (dev->pm_domain) {
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ }
return err;
}
@@ -439,6 +449,10 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
list_for_each_entry_safe(port, tmp, &pcie->ports, list)
mtk_pcie_enable_ports(port);
+ /* power down PCIe subsys if slots are all empty(link down) */
+ if (list_empty(&pcie->ports))
+ mtk_pcie_subsys_powerdown(pcie);
+
return 0;
}
@@ -522,7 +536,9 @@ static int mtk_pcie_probe(struct platform_device *pdev)
return 0;
put_resources:
- mtk_pcie_put_resources(pcie);
+ if (!list_empty(&pcie->ports))
+ mtk_pcie_put_resources(pcie);
+
return err;
}
--
1.9.1
The current powerup logic may lead to unbalanced pm_runtime_enable() if
the earlier probe failed due to -EPROBE_DEFER. Hence we do a little flow
adjustment to avoid that:
Parse standard PCI resources firstly, then check properties for each port,
power on controller finally.
Also, this patch removes unused registers and renames funtions properly.
Signed-off-by: Ryder Lee <[email protected]>
---
drivers/pci/host/pcie-mediatek.c | 51 +++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 3baafa8..165d82d 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -34,8 +34,6 @@
/* PCIe per port registers */
#define PCIE_BAR0_SETUP 0x10
-#define PCIE_BAR1_SETUP 0x14
-#define PCIE_BAR0_MEM_BASE 0x18
#define PCIE_CLASS 0x34
#define PCIE_LINK_STATUS 0x50
@@ -224,7 +222,7 @@ static void mtk_pcie_assert_ports(struct mtk_pcie_port *port)
msleep(100);
}
-static int mtk_pcie_enable_ports(struct mtk_pcie_port *port)
+static void mtk_pcie_enable_ports(struct mtk_pcie_port *port)
{
struct device *dev = port->pcie->dev;
int err;
@@ -249,7 +247,7 @@ static int mtk_pcie_enable_ports(struct mtk_pcie_port *port)
/* if link up, then setup root port configuration space */
if (mtk_pcie_link_is_up(port)) {
mtk_pcie_configure_rc(port);
- return 0;
+ return;
}
dev_info(dev, "Port%d link down\n", port->index);
@@ -257,13 +255,13 @@ static int mtk_pcie_enable_ports(struct mtk_pcie_port *port)
phy_power_off(port->phy);
err_phy_on:
clk_disable_unprepare(port->sys_ck);
- mtk_pcie_port_free(port);
err_sys_clk:
- return err;
+ mtk_pcie_port_free(port);
+
+ return;
}
static int mtk_pcie_parse_ports(struct mtk_pcie *pcie,
- struct mtk_pcie_port **p,
struct device_node *node,
int index)
{
@@ -274,12 +272,10 @@ static int mtk_pcie_parse_ports(struct mtk_pcie *pcie,
char name[10];
int err;
- *p = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
- if (!*p)
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
return -ENOMEM;
- port = *p;
-
err = of_property_read_u32(node, "num-lanes", &port->lane);
if (err) {
dev_err(dev, "missing num-lanes property\n");
@@ -323,7 +319,7 @@ static int mtk_pcie_parse_ports(struct mtk_pcie *pcie,
return 0;
}
-static int mtk_pcie_handle_shared_resource(struct mtk_pcie *pcie)
+static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
{
struct device *dev = pcie->dev;
struct platform_device *pdev = to_platform_device(dev);
@@ -366,20 +362,16 @@ static int mtk_pcie_handle_shared_resource(struct mtk_pcie *pcie)
return err;
}
-static int mtk_pcie_parse_and_add_res(struct mtk_pcie *pcie)
+static int mtk_pcie_setup(struct mtk_pcie *pcie)
{
struct device *dev = pcie->dev;
struct device_node *node = dev->of_node, *child;
struct of_pci_range_parser parser;
struct of_pci_range range;
struct resource res;
+ struct mtk_pcie_port *port, *tmp;
int err;
- /* parse shared resources */
- err = mtk_pcie_handle_shared_resource(pcie);
- if (err)
- return err;
-
if (of_pci_range_parser_init(&parser, node)) {
dev_err(dev, "missing \"ranges\" property\n");
return -EINVAL;
@@ -423,8 +415,7 @@ static int mtk_pcie_parse_and_add_res(struct mtk_pcie *pcie)
pcie->busn.flags = IORESOURCE_BUS;
}
- for_each_child_of_node(node, child) {
- struct mtk_pcie_port *port;
+ for_each_available_child_of_node(node, child) {
int index;
err = of_pci_get_devfn(child);
@@ -435,19 +426,19 @@ static int mtk_pcie_parse_and_add_res(struct mtk_pcie *pcie)
index = PCI_SLOT(err);
- if (!of_device_is_available(child))
- continue;
-
- err = mtk_pcie_parse_ports(pcie, &port, child, index);
- if (err)
- return err;
-
- /* enable each port, and then check link status */
- err = mtk_pcie_enable_ports(port);
+ err = mtk_pcie_parse_ports(pcie, child, index);
if (err)
return err;
}
+ err = mtk_pcie_subsys_powerup(pcie);
+ if (err)
+ return err;
+
+ /* enable each port, and then check link status */
+ list_for_each_entry_safe(port, tmp, &pcie->ports, list)
+ mtk_pcie_enable_ports(port);
+
return 0;
}
@@ -516,7 +507,7 @@ static int mtk_pcie_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcie);
INIT_LIST_HEAD(&pcie->ports);
- err = mtk_pcie_parse_and_add_res(pcie);
+ err = mtk_pcie_setup(pcie);
if (err)
return err;
--
1.9.1
Some properties for PCIe controller are optional and therefore change
to use *optional* function.
Signed-off-by: Ryder Lee <[email protected]>
---
drivers/pci/host/pcie-mediatek.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index d256263..91a6328 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -306,18 +306,15 @@ static int mtk_pcie_parse_ports(struct mtk_pcie *pcie,
}
snprintf(name, sizeof(name), "pcie-rst%d", index);
- port->reset = devm_reset_control_get(dev, name);
- if (IS_ERR(port->reset)) {
- dev_err(dev, "failed to get port%d reset\n", index);
+ port->reset = devm_reset_control_get_optional(dev, name);
+ if (PTR_ERR(port->reset) == -EPROBE_DEFER)
return PTR_ERR(port->reset);
- }
+ /* some platforms may use default PHY setting */
snprintf(name, sizeof(name), "pcie-phy%d", index);
- port->phy = devm_phy_get(dev, name);
- if (IS_ERR(port->phy)) {
- dev_err(dev, "failed to get port%d phy\n", index);
+ port->phy = devm_phy_optional_get(dev, name);
+ if (IS_ERR(port->phy))
return PTR_ERR(port->phy);
- }
port->index = index;
port->pcie = pcie;
@@ -345,8 +342,10 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
pcie->free_ck = devm_clk_get(dev, "free_ck");
if (IS_ERR(pcie->free_ck)) {
- dev_err(dev, "failed to get free_ck\n");
- return PTR_ERR(pcie->free_ck);
+ if (PTR_ERR(pcie->free_ck) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ pcie->free_ck = NULL;
}
if (dev->pm_domain) {
--
1.9.1
On Mon, Jun 19, 2017 at 05:59:57PM +0800, Ryder Lee wrote:
> Hi Bjorn,
>
> This patch series does some modifications to powerup logic to avoid unbalanced
> PM warnings, and turns off host power if no link is detected.
>
> I also make some properties(eg, PHY/reset/..) optional to gain more flexibility
> just in case future SoCs don't have it.
>
> Could you also fold this patch series into pci/host-mediatek for v4.13?
> Thanks!
>
> Ryder Lee (3):
> PCI: mediatek: modify controller powerup logic
> PCI: mediatek: turn off subsys power if no link up detected
> PCI: mediatek: make some properties optioanl
>
> drivers/pci/host/pcie-mediatek.c | 110 +++++++++++++++++++++------------------
> 1 file changed, 58 insertions(+), 52 deletions(-)
I folded these into the initial driver commit on pci/host-mediatek
for v4.13, thanks!