2024-03-04 15:36:28

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 00/18] Add suspend to ram support for PCIe on J7200

This add suspend to ram support for the PCIe (RC mode) on J7200 platform.

In RC mode, the reset pin for endpoints is managed by a gpio expander on a
i2c bus. This pin shall be managed in suspend_noirq() and resume_noirq().
The suspend/resume has been moved to suspend_noirq()/resume_noirq() for
pca953x (expander) and pinctrl-single.

To do i2c accesses during suspend_noirq/resume_noirq, we need to force the
wakeup of the i2c controller (which is autosuspended) during suspend
callback.
It's the only way to wakeup the controller if it's autosuspended, as
runtime pm is disabled in suspend_noirq and resume_noirq.

The main change in the v4 is that now mux_chip_resume() returns zero or on
the first error.
The 2 patches on the pinctrl-single driver were squashed.
And the subject line for the PCI patches was fixed.
Other changes are detailed below.

Regards,

Thomas

Signed-off-by: Thomas Richard <[email protected]>
---
Changes in v4:
- all: use SoB/Co-developed-by for patches initially developed by Théo
Lebrun.
- pinctrl-single: squash the two commits.
- i2c-omap: fix line lenghts of the comment in omap_i2c_suspend().
- mux: mux_chip_resume() return 0 or at the first error.
- phy-j721e-wiz: clean code around dev_err_probe().
- phy-j721e-wiz: use REF_CLK_100MHZ macros.
- pci: fix subject line for all PCI patches.
- pci-cadence: use fsleep() instead of usleep_range().
- pci-cadence: remove cdns_torrent_clk_cleanup() call in
cdns_torrent_phy_resume_noirq().
- pci-j721e: add a patch to use dev_err_probe() instead of dev_err() in the probe().
- pci-j721e: fix unordered header files.
- pci-j721e: remove some log spammers.
- pci-j721e: add a missing clock disable in j721e_pcie_resume_noirq().
- pci-j721e: simplify the patch "Add reset GPIO to struct j721e_pcie"
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- pinctrl-single: split patch in two parts, a first patch to remove the
dead code, a second to move suspend()/resume() callbacks to noirq.
- i2c-omap: add a comments above the suspend_noirq() callback.
- mux: now mux_chip_resume() try to restores all muxes, then return 0 if
all is ok or the first failure.
- mmio: fix commit message.
- phy-j721e-wiz: add a patch to use dev_err_probe() instead of dev_err() in
the wiz_clock_init() function.
- phy-j721e-wiz: remove probe boolean for the wiz_clock_init(), rename the
function to wiz_clock_probe(), extract hardware configuration part in a
new function wiz_clock_init().
- phy-cadence-torrent: use dev_err_probe() and fix commit messages
- pcie-cadence-host: remove probe boolean for the cdns_pcie_host_setup()
function and extract the link setup part in a new function
cdns_pcie_host_link_setup().
- pcie-cadence-host: make cdns_pcie_host_init() global.
- pci-j721e: use the cdns_pcie_host_link_setup() cdns_pcie_host_init()
functions in the resume_noirq() callback.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- all: fix commits messages.
- all: use DEFINE_NOIRQ_DEV_PM_OPS and pm_sleep_ptr macros.
- all: remove useless #ifdef CONFIG_PM.
- pinctrl-single: drop dead code
- mux: add mux_chip_resume() function in mux core.
- mmio: resume sequence is now a call to mux_chip_resume().
- phy-cadence-torrent: fix typo in resume sequence (reset_control_assert()
instead of reset_control_put()).
- phy-cadence-torrent: use PHY instead of phy.
- pci-j721e: do not shadow cdns_pcie_host_setup return code in resume
sequence.
- pci-j721e: drop dead code.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Thomas Richard (15):
gpio: pca953x: move suspend()/resume() to suspend_noirq()/resume_noirq()
pinctrl: pinctrl-single: move suspend()/resume() callbacks to noirq
i2c: omap: wakeup the controller during suspend() callback
mux: add mux_chip_resume() function
phy: ti: phy-j721e-wiz: use dev_err_probe() instead of dev_err()
phy: ti: phy-j721e-wiz: split wiz_clock_init() function
phy: ti: phy-j721e-wiz: add resume support
phy: cadence-torrent: extract calls to clk_get from cdns_torrent_clk
phy: cadence-torrent: register resets even if the phy is already configured
phy: cadence-torrent: add already_configured to struct cdns_torrent_phy
phy: cadence-torrent: remove noop_ops phy operations
phy: cadence-torrent: add suspend and resume support
PCI: cadence: Extract link setup sequence from cdns_pcie_host_setup()
PCI: cadence: Set cdns_pcie_host_init() global
PCI: j721e: Use dev_err_probe() in the probe() function

Théo Lebrun (3):
mux: mmio: add resume support
PCI: j721e: Add reset GPIO to struct j721e_pcie
PCI: j721e: Add suspend and resume support

drivers/gpio/gpio-pca953x.c | 7 +-
drivers/i2c/busses/i2c-omap.c | 22 ++++
drivers/mux/core.c | 29 +++++
drivers/mux/mmio.c | 12 ++
drivers/pci/controller/cadence/pci-j721e.c | 107 +++++++++++++++--
drivers/pci/controller/cadence/pcie-cadence-host.c | 44 ++++---
drivers/pci/controller/cadence/pcie-cadence.h | 12 ++
drivers/phy/cadence/phy-cadence-torrent.c | 120 ++++++++++++++-----
drivers/phy/ti/phy-j721e-wiz.c | 132 +++++++++++++--------
drivers/pinctrl/pinctrl-single.c | 28 ++---
include/linux/mux/driver.h | 1 +
11 files changed, 384 insertions(+), 130 deletions(-)
---
base-commit: e76807cc177efce5bdb253de3c673b13c08be013
change-id: 20240102-j7200-pcie-s2r-ecb1a979e357

Best regards,
--
Thomas Richard <[email protected]>



2024-03-04 15:37:49

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 03/18] i2c: omap: wakeup the controller during suspend() callback

A device may need the controller up during suspend_noirq() or
resume_noirq().
But if the controller is autosuspended, there is no way to wakeup it during
suspend_noirq() or resume_noirq() because runtime pm is disabled at this
time.

The suspend() callback wakes up the controller, so it is available until
its suspend_noirq() callback (pm_runtime_force_suspend()).
During the resume, it's restored by resume_noirq() callback
(pm_runtime_force_resume()). Then resume() callback enables autosuspend.

So the controller is up during a little time slot in suspend and resume
sequences even if it's not used.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/i2c/busses/i2c-omap.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 42165ef57946..28417b2a18b0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1575,9 +1575,31 @@ static int __maybe_unused omap_i2c_runtime_resume(struct device *dev)
return 0;
}

+static int omap_i2c_suspend(struct device *dev)
+{
+ /*
+ * If the controller is autosuspended, there is no way to wakeup it once
+ * runtime pm is disabled (in suspend_late()).
+ * But a device may need the controller up during suspend_noirq() or
+ * resume_noirq().
+ * Wakeup the controller while runtime pm is enabled, so it is available
+ * until its suspend_noirq(), and from resume_noirq().
+ */
+ return pm_runtime_resume_and_get(dev);
+}
+
+static int omap_i2c_resume(struct device *dev)
+{
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+ return 0;
+}
+
static const struct dev_pm_ops omap_i2c_pm_ops = {
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(omap_i2c_suspend, omap_i2c_resume)
SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,
omap_i2c_runtime_resume, NULL)
};

--
2.39.2


2024-03-04 15:38:47

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 01/18] gpio: pca953x: move suspend()/resume() to suspend_noirq()/resume_noirq()

Some IOs can be needed during suspend_noirq()/resume_noirq().
So move suspend()/resume() to noirq.

Reviewed-by: Andi Shyti <[email protected]>
Acked-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/gpio/gpio-pca953x.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 00ffa168e405..6e495fc67a93 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -1234,7 +1234,7 @@ static void pca953x_save_context(struct pca953x_chip *chip)
regcache_cache_only(chip->regmap, true);
}

-static int pca953x_suspend(struct device *dev)
+static int pca953x_suspend_noirq(struct device *dev)
{
struct pca953x_chip *chip = dev_get_drvdata(dev);

@@ -1248,7 +1248,7 @@ static int pca953x_suspend(struct device *dev)
return 0;
}

-static int pca953x_resume(struct device *dev)
+static int pca953x_resume_noirq(struct device *dev)
{
struct pca953x_chip *chip = dev_get_drvdata(dev);
int ret;
@@ -1268,7 +1268,8 @@ static int pca953x_resume(struct device *dev)
return ret;
}

-static DEFINE_SIMPLE_DEV_PM_OPS(pca953x_pm_ops, pca953x_suspend, pca953x_resume);
+static DEFINE_NOIRQ_DEV_PM_OPS(pca953x_pm_ops,
+ pca953x_suspend_noirq, pca953x_resume_noirq);

/* convenience to stop overlong match-table lines */
#define OF_653X(__nrgpio, __int) ((void *)(__nrgpio | PCAL653X_TYPE | __int))

--
2.39.2


2024-03-04 15:39:26

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 06/18] phy: ti: phy-j721e-wiz: use dev_err_probe() instead of dev_err()

Use dev_err_probe() instead of dev_err() in wiz_clock_init() to simplify
the code and standardize the error output.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/ti/phy-j721e-wiz.c | 46 +++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index 00d7e6a6de03..5fea4df9404e 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -1088,11 +1088,10 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
int i;

clk = devm_clk_get(dev, "core_ref_clk");
- if (IS_ERR(clk)) {
- dev_err(dev, "core_ref_clk clock not found\n");
- ret = PTR_ERR(clk);
- return ret;
- }
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "core_ref_clk clock not found\n");
+
wiz->input_clks[WIZ_CORE_REFCLK] = clk;

rate = clk_get_rate(clk);
@@ -1122,11 +1121,10 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)

if (wiz->data->pma_cmn_refclk1_int_mode) {
clk = devm_clk_get(dev, "core_ref1_clk");
- if (IS_ERR(clk)) {
- dev_err(dev, "core_ref1_clk clock not found\n");
- ret = PTR_ERR(clk);
- return ret;
- }
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "core_ref1_clk clock not found\n");
+
wiz->input_clks[WIZ_CORE_REFCLK1] = clk;

rate = clk_get_rate(clk);
@@ -1137,11 +1135,10 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
}

clk = devm_clk_get(dev, "ext_ref_clk");
- if (IS_ERR(clk)) {
- dev_err(dev, "ext_ref_clk clock not found\n");
- ret = PTR_ERR(clk);
- return ret;
- }
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "ext_ref_clk clock not found\n");
+
wiz->input_clks[WIZ_EXT_REFCLK] = clk;

rate = clk_get_rate(clk);
@@ -1157,8 +1154,9 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
case J721S2_WIZ_10G:
ret = wiz_clock_register(wiz);
if (ret)
- dev_err(dev, "Failed to register wiz clocks\n");
- return ret;
+ return dev_err_probe(dev, ret, "Failed to register wiz clocks\n");
+
+ return 0;
default:
break;
}
@@ -1167,16 +1165,15 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
node_name = clk_mux_sel[i].node_name;
clk_node = of_get_child_by_name(node, node_name);
if (!clk_node) {
- dev_err(dev, "Unable to get %s node\n", node_name);
- ret = -EINVAL;
+ ret = dev_err_probe(dev, -EINVAL, "Unable to get %s node\n", node_name);
goto err;
}

ret = wiz_mux_of_clk_register(wiz, clk_node, wiz->mux_sel_field[i],
clk_mux_sel[i].table);
if (ret) {
- dev_err(dev, "Failed to register %s clock\n",
- node_name);
+ dev_err_probe(dev, ret, "Failed to register %s clock\n",
+ node_name);
of_node_put(clk_node);
goto err;
}
@@ -1188,16 +1185,15 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
node_name = clk_div_sel[i].node_name;
clk_node = of_get_child_by_name(node, node_name);
if (!clk_node) {
- dev_err(dev, "Unable to get %s node\n", node_name);
- ret = -EINVAL;
+ ret = dev_err_probe(dev, -EINVAL, "Unable to get %s node\n", node_name);
goto err;
}

ret = wiz_div_clk_register(wiz, clk_node, wiz->div_sel_field[i],
clk_div_sel[i].table);
if (ret) {
- dev_err(dev, "Failed to register %s clock\n",
- node_name);
+ dev_err_probe(dev, ret, "Failed to register %s clock\n",
+ node_name);
of_node_put(clk_node);
goto err;
}

--
2.39.2


2024-03-04 15:39:34

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 07/18] phy: ti: phy-j721e-wiz: split wiz_clock_init() function

The wiz_clock_init() function mixes probe and hardware configuration.
Rename the wiz_clock_init() to wiz_clock_probe() and move the hardware
configuration part in a new function named wiz_clock_init().

This hardware configuration sequence must be called during the resume
stage of the driver.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/ti/phy-j721e-wiz.c | 67 ++++++++++++++++++++++++------------------
1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index 5fea4df9404e..0e3cb1ed5a52 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -1076,26 +1076,12 @@ static int wiz_clock_register(struct wiz *wiz)
return ret;
}

-static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
+static void wiz_clock_init(struct wiz *wiz)
{
- const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
- struct device *dev = wiz->dev;
- struct device_node *clk_node;
- const char *node_name;
unsigned long rate;
- struct clk *clk;
- int ret;
- int i;
-
- clk = devm_clk_get(dev, "core_ref_clk");
- if (IS_ERR(clk))
- return dev_err_probe(dev, PTR_ERR(clk),
- "core_ref_clk clock not found\n");

- wiz->input_clks[WIZ_CORE_REFCLK] = clk;
-
- rate = clk_get_rate(clk);
- if (rate >= 100000000)
+ rate = clk_get_rate(wiz->input_clks[WIZ_CORE_REFCLK]);
+ if (rate >= REF_CLK_100MHZ)
regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x1);
else
regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x3);
@@ -1119,6 +1105,39 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
break;
}

+ if (wiz->input_clks[WIZ_CORE_REFCLK1]) {
+ rate = clk_get_rate(wiz->input_clks[WIZ_CORE_REFCLK1]);
+ if (rate >= REF_CLK_100MHZ)
+ regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
+ else
+ regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
+
+ }
+
+ rate = clk_get_rate(wiz->input_clks[WIZ_EXT_REFCLK]);
+ if (rate >= REF_CLK_100MHZ)
+ regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
+ else
+ regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
+}
+
+static int wiz_clock_probe(struct wiz *wiz, struct device_node *node)
+{
+ const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
+ struct device *dev = wiz->dev;
+ struct device_node *clk_node;
+ const char *node_name;
+ struct clk *clk;
+ int ret;
+ int i;
+
+ clk = devm_clk_get(dev, "core_ref_clk");
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "core_ref_clk clock not found\n");
+
+ wiz->input_clks[WIZ_CORE_REFCLK] = clk;
+
if (wiz->data->pma_cmn_refclk1_int_mode) {
clk = devm_clk_get(dev, "core_ref1_clk");
if (IS_ERR(clk))
@@ -1126,12 +1145,6 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
"core_ref1_clk clock not found\n");

wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
-
- rate = clk_get_rate(clk);
- if (rate >= 100000000)
- regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
- else
- regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
}

clk = devm_clk_get(dev, "ext_ref_clk");
@@ -1141,11 +1154,7 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)

wiz->input_clks[WIZ_EXT_REFCLK] = clk;

- rate = clk_get_rate(clk);
- if (rate >= 100000000)
- regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
- else
- regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
+ wiz_clock_init(wiz);

switch (wiz->type) {
case AM64_WIZ_10G:
@@ -1589,7 +1598,7 @@ static int wiz_probe(struct platform_device *pdev)
goto err_get_sync;
}

- ret = wiz_clock_init(wiz, node);
+ ret = wiz_clock_probe(wiz, node);
if (ret < 0) {
dev_warn(dev, "Failed to initialize clocks\n");
goto err_get_sync;

--
2.39.2


2024-03-04 15:39:42

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 08/18] phy: ti: phy-j721e-wiz: add resume support

Add resume support.
It has been tested on J7200 SR1.0 and SR2.0.

Co-developed-by: Théo Lebrun <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/ti/phy-j721e-wiz.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index 0e3cb1ed5a52..b2320f2efb72 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -1660,12 +1660,41 @@ static void wiz_remove(struct platform_device *pdev)
pm_runtime_disable(dev);
}

+static int wiz_resume_noirq(struct device *dev)
+{
+ struct device_node *node = dev->of_node;
+ struct wiz *wiz = dev_get_drvdata(dev);
+ int ret;
+
+ /* Enable supplemental Control override if available */
+ if (wiz->sup_legacy_clk_override)
+ regmap_field_write(wiz->sup_legacy_clk_override, 1);
+
+ wiz_clock_init(wiz);
+
+ ret = wiz_init(wiz);
+ if (ret) {
+ dev_err(dev, "WIZ initialization failed\n");
+ goto err_wiz_init;
+ }
+
+ return 0;
+
+err_wiz_init:
+ wiz_clock_cleanup(wiz, node);
+
+ return ret;
+}
+
+static DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, NULL, wiz_resume_noirq);
+
static struct platform_driver wiz_driver = {
.probe = wiz_probe,
.remove_new = wiz_remove,
.driver = {
.name = "wiz",
.of_match_table = wiz_id_table,
+ .pm = pm_sleep_ptr(&wiz_pm_ops),
},
};
module_platform_driver(wiz_driver);

--
2.39.2


2024-03-04 15:40:02

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 10/18] phy: cadence-torrent: register resets even if the phy is already configured

Resets are needed during suspend and resume stages.
So they shall be registered during the probe even the phy is already
initialized.

The function cdns_torrent_reset is renamed cdns_torrent_of_get_reset() to
make it clear.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/cadence/phy-cadence-torrent.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
index 803a76acf2fd..bba10ca0bfdd 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -2660,7 +2660,7 @@ static int cdns_torrent_clk_register(struct cdns_torrent_phy *cdns_phy)
return 0;
}

-static int cdns_torrent_reset(struct cdns_torrent_phy *cdns_phy)
+static int cdns_torrent_of_get_reset(struct cdns_torrent_phy *cdns_phy)
{
struct device *dev = cdns_phy->dev;

@@ -2779,6 +2779,10 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
if (ret)
return ret;

+ ret = cdns_torrent_of_get_reset(cdns_phy);
+ if (ret)
+ goto clk_cleanup;
+
ret = cdns_torrent_of_get_clk(cdns_phy);
if (ret)
goto clk_cleanup;
@@ -2786,10 +2790,6 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1, &already_configured);

if (!already_configured) {
- ret = cdns_torrent_reset(cdns_phy);
- if (ret)
- goto clk_cleanup;
-
ret = cdns_torrent_clk(cdns_phy);
if (ret)
goto clk_cleanup;

--
2.39.2


2024-03-04 15:40:44

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 04/18] mux: add mux_chip_resume() function

The mux_chip_resume() function restores a mux_chip using the cached state
of each mux.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/mux/core.c | 29 +++++++++++++++++++++++++++++
include/linux/mux/driver.h | 1 +
2 files changed, 30 insertions(+)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 775816112932..0742aa2a7c73 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -215,6 +215,35 @@ void mux_chip_free(struct mux_chip *mux_chip)
}
EXPORT_SYMBOL_GPL(mux_chip_free);

+/**
+ * mux_chip_resume() - restores the mux-chip state
+ * @mux_chip: The mux-chip to resume.
+ *
+ * Restores the mux-chip state.
+ *
+ * Return: Zero on success or a negative errno on error.
+ */
+int mux_chip_resume(struct mux_chip *mux_chip)
+{
+ int ret, i;
+
+ for (i = 0; i < mux_chip->controllers; ++i) {
+ struct mux_control *mux = &mux_chip->mux[i];
+
+ if (mux->cached_state == MUX_CACHE_UNKNOWN)
+ continue;
+
+ ret = mux_control_set(mux, mux->cached_state);
+ if (ret < 0) {
+ dev_err(&mux_chip->dev, "unable to restore state\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mux_chip_resume);
+
static void devm_mux_chip_release(struct device *dev, void *res)
{
struct mux_chip *mux_chip = *(struct mux_chip **)res;
diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
index 18824064f8c0..2a7e5ec5d540 100644
--- a/include/linux/mux/driver.h
+++ b/include/linux/mux/driver.h
@@ -88,6 +88,7 @@ struct mux_chip *mux_chip_alloc(struct device *dev,
int mux_chip_register(struct mux_chip *mux_chip);
void mux_chip_unregister(struct mux_chip *mux_chip);
void mux_chip_free(struct mux_chip *mux_chip);
+int mux_chip_resume(struct mux_chip *mux_chip);

struct mux_chip *devm_mux_chip_alloc(struct device *dev,
unsigned int controllers,

--
2.39.2


2024-03-04 15:40:51

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 13/18] phy: cadence-torrent: add suspend and resume support

Add suspend and resume support.

The already_configured flag is cleared during the suspend stage to force
the PHY initialization during the resume stage.

Co-developed-by: Théo Lebrun <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/cadence/phy-cadence-torrent.c | 53 +++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
index 52cadca4c07b..b4245c088760 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -3005,6 +3005,58 @@ static void cdns_torrent_phy_remove(struct platform_device *pdev)
cdns_torrent_clk_cleanup(cdns_phy);
}

+static int cdns_torrent_phy_suspend_noirq(struct device *dev)
+{
+ struct cdns_torrent_phy *cdns_phy = dev_get_drvdata(dev);
+ int i;
+
+ reset_control_assert(cdns_phy->phy_rst);
+ reset_control_assert(cdns_phy->apb_rst);
+ for (i = 0; i < cdns_phy->nsubnodes; i++)
+ reset_control_assert(cdns_phy->phys[i].lnk_rst);
+
+ if (cdns_phy->already_configured)
+ cdns_phy->already_configured = 0;
+ else
+ clk_disable_unprepare(cdns_phy->clk);
+
+ return 0;
+}
+
+static int cdns_torrent_phy_resume_noirq(struct device *dev)
+{
+ struct cdns_torrent_phy *cdns_phy = dev_get_drvdata(dev);
+ int node = cdns_phy->nsubnodes;
+ int ret, i;
+
+ ret = cdns_torrent_clk(cdns_phy);
+ if (ret)
+ return ret;
+
+ /* Enable APB */
+ reset_control_deassert(cdns_phy->apb_rst);
+
+ if (cdns_phy->nsubnodes > 1) {
+ ret = cdns_torrent_phy_configure_multilink(cdns_phy);
+ if (ret)
+ goto put_lnk_rst;
+ }
+
+ return 0;
+
+put_lnk_rst:
+ for (i = 0; i < node; i++)
+ reset_control_assert(cdns_phy->phys[i].lnk_rst);
+ reset_control_assert(cdns_phy->apb_rst);
+ clk_disable_unprepare(cdns_phy->clk);
+
+ return ret;
+}
+
+static DEFINE_NOIRQ_DEV_PM_OPS(cdns_torrent_phy_pm_ops,
+ cdns_torrent_phy_suspend_noirq,
+ cdns_torrent_phy_resume_noirq);
+
/* USB and DP link configuration */
static struct cdns_reg_pairs usb_dp_link_cmn_regs[] = {
{0x0002, PHY_PLL_CFG},
@@ -4576,6 +4628,7 @@ static struct platform_driver cdns_torrent_phy_driver = {
.driver = {
.name = "cdns-torrent-phy",
.of_match_table = cdns_torrent_phy_of_match,
+ .pm = pm_sleep_ptr(&cdns_torrent_phy_pm_ops),
}
};
module_platform_driver(cdns_torrent_phy_driver);

--
2.39.2


2024-03-04 15:41:00

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 14/18] PCI: cadence: Extract link setup sequence from cdns_pcie_host_setup()

The function cdns_pcie_host_setup() mixes probe structure and link setup.

The link setup must be done during the resume sequence. So extract it from
cdns_pcie_host_setup() and create a dedicated function.

Reviewed-by: Siddharth Vadapalli <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/pci/controller/cadence/pcie-cadence-host.c | 39 ++++++++++++++--------
drivers/pci/controller/cadence/pcie-cadence.h | 6 ++++
2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 5b14f7ee3c79..93d9922730af 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -497,6 +497,30 @@ static int cdns_pcie_host_init(struct device *dev,
return cdns_pcie_host_init_address_translation(rc);
}

+int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc)
+{
+ struct cdns_pcie *pcie = &rc->pcie;
+ struct device *dev = rc->pcie.dev;
+ int ret;
+
+ if (rc->quirk_detect_quiet_flag)
+ cdns_pcie_detect_quiet_min_delay_set(&rc->pcie);
+
+ cdns_pcie_host_enable_ptm_response(pcie);
+
+ ret = cdns_pcie_start_link(pcie);
+ if (ret) {
+ dev_err(dev, "Failed to start link\n");
+ return ret;
+ }
+
+ ret = cdns_pcie_host_start_link(rc);
+ if (ret)
+ dev_dbg(dev, "PCIe link never came up\n");
+
+ return 0;
+}
+
int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
{
struct device *dev = rc->pcie.dev;
@@ -533,20 +557,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
return PTR_ERR(rc->cfg_base);
rc->cfg_res = res;

- if (rc->quirk_detect_quiet_flag)
- cdns_pcie_detect_quiet_min_delay_set(&rc->pcie);
-
- cdns_pcie_host_enable_ptm_response(pcie);
-
- ret = cdns_pcie_start_link(pcie);
- if (ret) {
- dev_err(dev, "Failed to start link\n");
- return ret;
- }
-
- ret = cdns_pcie_host_start_link(rc);
+ ret = cdns_pcie_host_link_setup(rc);
if (ret)
- dev_dbg(dev, "PCIe link never came up\n");
+ return ret;

for (bar = RP_BAR0; bar <= RP_NO_BAR; bar++)
rc->avail_ib_bar[bar] = true;
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 03b96798f858..9ca2ffca0bf9 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -515,10 +515,16 @@ static inline bool cdns_pcie_link_up(struct cdns_pcie *pcie)
}

#ifdef CONFIG_PCIE_CADENCE_HOST
+int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc);
int cdns_pcie_host_setup(struct cdns_pcie_rc *rc);
void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
int where);
#else
+static inline int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc)
+{
+ return 0;
+}
+
static inline int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
{
return 0;

--
2.39.2


2024-03-04 15:41:11

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 15/18] PCI: cadence: Set cdns_pcie_host_init() global

During the resume sequence of the host, cdns_pcie_host_init() needs to be
called, so set it global.

The dev function parameter is removed, as it isn't used.

Reviewed-by: Siddharth Vadapalli <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/pci/controller/cadence/pcie-cadence-host.c | 5 ++---
drivers/pci/controller/cadence/pcie-cadence.h | 6 ++++++
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 93d9922730af..8af95e9da7ce 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -485,8 +485,7 @@ static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
return cdns_pcie_host_map_dma_ranges(rc);
}

-static int cdns_pcie_host_init(struct device *dev,
- struct cdns_pcie_rc *rc)
+int cdns_pcie_host_init(struct cdns_pcie_rc *rc)
{
int err;

@@ -564,7 +563,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
for (bar = RP_BAR0; bar <= RP_NO_BAR; bar++)
rc->avail_ib_bar[bar] = true;

- ret = cdns_pcie_host_init(dev, rc);
+ ret = cdns_pcie_host_init(rc);
if (ret)
return ret;

diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 9ca2ffca0bf9..a2e04261237d 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -516,6 +516,7 @@ static inline bool cdns_pcie_link_up(struct cdns_pcie *pcie)

#ifdef CONFIG_PCIE_CADENCE_HOST
int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc);
+int cdns_pcie_host_init(struct cdns_pcie_rc *rc);
int cdns_pcie_host_setup(struct cdns_pcie_rc *rc);
void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
int where);
@@ -525,6 +526,11 @@ static inline int cdns_pcie_host_link_setup(struct cdns_pcie_rc *rc)
return 0;
}

+static inline int cdns_pcie_host_init(struct cdns_pcie_rc *rc)
+{
+ return 0;
+}
+
static inline int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
{
return 0;

--
2.39.2


2024-03-04 15:42:08

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 18/18] PCI: j721e: Add suspend and resume support

From: Théo Lebrun <[email protected]>

Add suspend and resume support. Only the rc mode is supported.

During the suspend stage PERST# is asserted, then deasserted during the
resume stage.

Signed-off-by: Théo Lebrun <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/pci/controller/cadence/pci-j721e.c | 86 ++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)

diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 9af4fd64c1f9..a1f1232e8ee5 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -7,6 +7,8 @@
*/

#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/container_of.h>
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
#include <linux/io.h>
@@ -22,6 +24,8 @@
#include "../../pci.h"
#include "pcie-cadence.h"

+#define cdns_pcie_to_rc(p) container_of(p, struct cdns_pcie_rc, pcie)
+
#define ENABLE_REG_SYS_2 0x108
#define STATUS_REG_SYS_2 0x508
#define STATUS_CLR_REG_SYS_2 0x708
@@ -588,6 +592,87 @@ static void j721e_pcie_remove(struct platform_device *pdev)
pm_runtime_disable(dev);
}

+static int j721e_pcie_suspend_noirq(struct device *dev)
+{
+ struct j721e_pcie *pcie = dev_get_drvdata(dev);
+
+ if (pcie->mode == PCI_MODE_RC) {
+ gpiod_set_value_cansleep(pcie->reset_gpio, 0);
+ clk_disable_unprepare(pcie->refclk);
+ }
+
+ cdns_pcie_disable_phy(pcie->cdns_pcie);
+
+ return 0;
+}
+
+static int j721e_pcie_resume_noirq(struct device *dev)
+{
+ struct j721e_pcie *pcie = dev_get_drvdata(dev);
+ struct cdns_pcie *cdns_pcie = pcie->cdns_pcie;
+ int ret;
+
+ ret = j721e_pcie_ctrl_init(pcie);
+ if (ret < 0)
+ return ret;
+
+ j721e_pcie_config_link_irq(pcie);
+
+ /*
+ * This is not called explicitly in the probe, it is called by
+ * cdns_pcie_init_phy().
+ */
+ ret = cdns_pcie_enable_phy(pcie->cdns_pcie);
+ if (ret < 0)
+ return ret;
+
+ if (pcie->mode == PCI_MODE_RC) {
+ struct cdns_pcie_rc *rc = cdns_pcie_to_rc(cdns_pcie);
+
+ ret = clk_prepare_enable(pcie->refclk);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * "Power Sequencing and Reset Signal Timings" table in
+ * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, REV. 3.0
+ * indicates PERST# should be deasserted after minimum of 100us
+ * once REFCLK is stable. The REFCLK to the connector in RC
+ * mode is selected while enabling the PHY. So deassert PERST#
+ * after 100 us.
+ */
+ if (pcie->reset_gpio) {
+ fsleep(100);
+ gpiod_set_value_cansleep(pcie->reset_gpio, 1);
+ }
+
+ ret = cdns_pcie_host_link_setup(rc);
+ if (ret < 0) {
+ clk_disable_unprepare(pcie->refclk);
+ return ret;
+ }
+
+ /*
+ * Reset internal status of BARs to force reinitialization in
+ * cdns_pcie_host_init().
+ */
+ for (enum cdns_pcie_rp_bar bar = RP_BAR0; bar <= RP_NO_BAR; bar++)
+ rc->avail_ib_bar[bar] = true;
+
+ ret = cdns_pcie_host_init(rc);
+ if (ret) {
+ clk_disable_unprepare(pcie->refclk);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static DEFINE_NOIRQ_DEV_PM_OPS(j721e_pcie_pm_ops,
+ j721e_pcie_suspend_noirq,
+ j721e_pcie_resume_noirq);
+
static struct platform_driver j721e_pcie_driver = {
.probe = j721e_pcie_probe,
.remove_new = j721e_pcie_remove,
@@ -595,6 +680,7 @@ static struct platform_driver j721e_pcie_driver = {
.name = "j721e-pcie",
.of_match_table = of_j721e_pcie_match,
.suppress_bind_attrs = true,
+ .pm = pm_sleep_ptr(&j721e_pcie_pm_ops),
},
};
builtin_platform_driver(j721e_pcie_driver);

--
2.39.2


2024-03-04 15:42:13

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 11/18] phy: cadence-torrent: add already_configured to struct cdns_torrent_phy

Add already_configured to struct cdns_torrent_phy, so it can be used at
differents stages.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/cadence/phy-cadence-torrent.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
index bba10ca0bfdd..b35fbc8a60e5 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -358,6 +358,7 @@ struct cdns_torrent_phy {
enum cdns_torrent_ref_clk ref_clk_rate;
struct cdns_torrent_inst phys[MAX_NUM_LANES];
int nsubnodes;
+ int already_configured;
const struct cdns_torrent_data *init_data;
struct regmap *regmap_common_cdb;
struct regmap *regmap_phy_pcs_common_cdb;
@@ -2740,7 +2741,6 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
struct device_node *child;
int ret, subnodes, node = 0, i;
u32 total_num_lanes = 0;
- int already_configured;
u8 init_dp_regmap = 0;
u32 phy_type;

@@ -2787,9 +2787,9 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
if (ret)
goto clk_cleanup;

- regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1, &already_configured);
+ regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1, &cdns_phy->already_configured);

- if (!already_configured) {
+ if (!cdns_phy->already_configured) {
ret = cdns_torrent_clk(cdns_phy);
if (ret)
goto clk_cleanup;
@@ -2869,7 +2869,7 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
of_property_read_u32(child, "cdns,ssc-mode",
&cdns_phy->phys[node].ssc_mode);

- if (!already_configured)
+ if (!cdns_phy->already_configured)
gphy = devm_phy_create(dev, child, &cdns_torrent_phy_ops);
else
gphy = devm_phy_create(dev, child, &noop_ops);
@@ -2955,7 +2955,7 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
goto put_lnk_rst;
}

- if (cdns_phy->nsubnodes > 1 && !already_configured) {
+ if (cdns_phy->nsubnodes > 1 && !cdns_phy->already_configured) {
ret = cdns_torrent_phy_configure_multilink(cdns_phy);
if (ret)
goto put_lnk_rst;

--
2.39.2


2024-03-04 15:43:19

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 16/18] PCI: j721e: Use dev_err_probe() in the probe() function

Use dev_err_probe() instead of dev_err() in the probe() function to
simplify the code and standardize the error output.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/pci/controller/cadence/pci-j721e.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 85718246016b..98484f001562 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -482,20 +482,20 @@ static int j721e_pcie_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
- dev_err(dev, "pm_runtime_get_sync failed\n");
+ dev_err_probe(dev, ret, "pm_runtime_get_sync failed\n");
goto err_get_sync;
}

ret = j721e_pcie_ctrl_init(pcie);
if (ret < 0) {
- dev_err(dev, "pm_runtime_get_sync failed\n");
+ dev_err_probe(dev, ret, "pm_runtime_get_sync failed\n");
goto err_get_sync;
}

ret = devm_request_irq(dev, irq, j721e_pcie_link_irq_handler, 0,
"j721e-pcie-link-down-irq", pcie);
if (ret < 0) {
- dev_err(dev, "failed to request link state IRQ %d\n", irq);
+ dev_err_probe(dev, ret, "failed to request link state IRQ %d\n", irq);
goto err_get_sync;
}

@@ -505,28 +505,25 @@ static int j721e_pcie_probe(struct platform_device *pdev)
case PCI_MODE_RC:
gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(gpiod)) {
- ret = PTR_ERR(gpiod);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to get reset GPIO\n");
+ ret = dev_err_probe(dev, PTR_ERR(gpiod), "Failed to get reset GPIO\n");
goto err_get_sync;
}

ret = cdns_pcie_init_phy(dev, cdns_pcie);
if (ret) {
- dev_err(dev, "Failed to init phy\n");
+ dev_err_probe(dev, ret, "Failed to init phy\n");
goto err_get_sync;
}

clk = devm_clk_get_optional(dev, "pcie_refclk");
if (IS_ERR(clk)) {
- ret = PTR_ERR(clk);
- dev_err(dev, "failed to get pcie_refclk\n");
+ ret = dev_err_probe(dev, PTR_ERR(clk), "failed to get pcie_refclk\n");
goto err_pcie_setup;
}

ret = clk_prepare_enable(clk);
if (ret) {
- dev_err(dev, "failed to enable pcie_refclk\n");
+ dev_err_probe(dev, ret, "failed to enable pcie_refclk\n");
goto err_pcie_setup;
}
pcie->refclk = clk;
@@ -554,7 +551,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
case PCI_MODE_EP:
ret = cdns_pcie_init_phy(dev, cdns_pcie);
if (ret) {
- dev_err(dev, "Failed to init phy\n");
+ dev_err_probe(dev, ret, "Failed to init phy\n");
goto err_get_sync;
}


--
2.39.2


2024-03-04 15:43:30

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 17/18] PCI: j721e: Add reset GPIO to struct j721e_pcie

From: Théo Lebrun <[email protected]>

Add reset GPIO to struct j721e_pcie, so it can be used at suspend and
resume stages.

Signed-off-by: Théo Lebrun <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/pci/controller/cadence/pci-j721e.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 98484f001562..9af4fd64c1f9 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -52,6 +52,7 @@ struct j721e_pcie {
u32 mode;
u32 num_lanes;
u32 max_lanes;
+ struct gpio_desc *reset_gpio;
void __iomem *user_cfg_base;
void __iomem *intd_cfg_base;
u32 linkdown_irq_regfield;
@@ -508,6 +509,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
ret = dev_err_probe(dev, PTR_ERR(gpiod), "Failed to get reset GPIO\n");
goto err_get_sync;
}
+ pcie->reset_gpio = gpiod;

ret = cdns_pcie_init_phy(dev, cdns_pcie);
if (ret) {

--
2.39.2


2024-03-04 16:07:29

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 02/18] pinctrl: pinctrl-single: move suspend()/resume() callbacks to noirq

The goal is to extend the active period of pinctrl.
Some devices may need active pinctrl after suspend() and/or before
resume().
So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
have active pinctrl until suspend_noirq() (included), and from
resume_noirq() (included).

The deprecated API has been removed to use the new one (dev_pm_ops struct).

No need to check the pointer returned by dev_get_drvdata(), as
platform_set_drvdata() is called during the probe.

Reviewed-by: Linus Walleij <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/pinctrl/pinctrl-single.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 19cc0db771a5..0dd4b0e11adf 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1625,7 +1625,6 @@ static int pcs_irq_init_chained_handler(struct pcs_device *pcs,
return 0;
}

-#ifdef CONFIG_PM
static int pcs_save_context(struct pcs_device *pcs)
{
int i, mux_bytes;
@@ -1690,14 +1689,9 @@ static void pcs_restore_context(struct pcs_device *pcs)
}
}

-static int pinctrl_single_suspend(struct platform_device *pdev,
- pm_message_t state)
+static int pinctrl_single_suspend_noirq(struct device *dev)
{
- struct pcs_device *pcs;
-
- pcs = platform_get_drvdata(pdev);
- if (!pcs)
- return -EINVAL;
+ struct pcs_device *pcs = dev_get_drvdata(dev);

if (pcs->flags & PCS_CONTEXT_LOSS_OFF) {
int ret;
@@ -1710,20 +1704,19 @@ static int pinctrl_single_suspend(struct platform_device *pdev,
return pinctrl_force_sleep(pcs->pctl);
}

-static int pinctrl_single_resume(struct platform_device *pdev)
+static int pinctrl_single_resume_noirq(struct device *dev)
{
- struct pcs_device *pcs;
-
- pcs = platform_get_drvdata(pdev);
- if (!pcs)
- return -EINVAL;
+ struct pcs_device *pcs = dev_get_drvdata(dev);

if (pcs->flags & PCS_CONTEXT_LOSS_OFF)
pcs_restore_context(pcs);

return pinctrl_force_default(pcs->pctl);
}
-#endif
+
+static DEFINE_NOIRQ_DEV_PM_OPS(pinctrl_single_pm_ops,
+ pinctrl_single_suspend_noirq,
+ pinctrl_single_resume_noirq);

/**
* pcs_quirk_missing_pinctrl_cells - handle legacy binding
@@ -1986,11 +1979,8 @@ static struct platform_driver pcs_driver = {
.driver = {
.name = DRIVER_NAME,
.of_match_table = pcs_of_match,
+ .pm = pm_sleep_ptr(&pinctrl_single_pm_ops),
},
-#ifdef CONFIG_PM
- .suspend = pinctrl_single_suspend,
- .resume = pinctrl_single_resume,
-#endif
};

module_platform_driver(pcs_driver);

--
2.39.2


2024-03-04 16:08:08

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 05/18] mux: mmio: add resume support

From: Théo Lebrun <[email protected]>

No need to save something during the suspend stage, as the mux core has an
internal cache to store the state of muxes.

This cache is used by mux_chip_resume() to restore all muxes.

Signed-off-by: Théo Lebrun <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/mux/mmio.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
index 30a952c34365..00405abe3ce3 100644
--- a/drivers/mux/mmio.c
+++ b/drivers/mux/mmio.c
@@ -130,13 +130,25 @@ static int mux_mmio_probe(struct platform_device *pdev)

mux_chip->ops = &mux_mmio_ops;

+ dev_set_drvdata(dev, mux_chip);
+
return devm_mux_chip_register(dev, mux_chip);
}

+static int mux_mmio_resume_noirq(struct device *dev)
+{
+ struct mux_chip *mux_chip = dev_get_drvdata(dev);
+
+ return mux_chip_resume(mux_chip);
+}
+
+static DEFINE_NOIRQ_DEV_PM_OPS(mux_mmio_pm_ops, NULL, mux_mmio_resume_noirq);
+
static struct platform_driver mux_mmio_driver = {
.driver = {
.name = "mmio-mux",
.of_match_table = mux_mmio_dt_ids,
+ .pm = pm_sleep_ptr(&mux_mmio_pm_ops),
},
.probe = mux_mmio_probe,
};

--
2.39.2


2024-03-04 16:09:58

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 09/18] phy: cadence-torrent: extract calls to clk_get from cdns_torrent_clk

Extract calls to clk_get from cdns_torrent_clk into a separate function.
It needs to call cdns_torrent_clk at resume without looking up the clock.

Co-developed-by: Théo Lebrun <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/cadence/phy-cadence-torrent.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
index a75c96385c57..803a76acf2fd 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -2681,18 +2681,21 @@ static int cdns_torrent_reset(struct cdns_torrent_phy *cdns_phy)
return 0;
}

+static int cdns_torrent_of_get_clk(struct cdns_torrent_phy *cdns_phy)
+{
+ cdns_phy->clk = devm_clk_get(cdns_phy->dev, "refclk");
+ if (IS_ERR(cdns_phy->clk))
+ return dev_err_probe(cdns_phy->dev, PTR_ERR(cdns_phy->clk),
+ "phy ref clock not found\n");
+
+ return 0;
+}
+
static int cdns_torrent_clk(struct cdns_torrent_phy *cdns_phy)
{
- struct device *dev = cdns_phy->dev;
unsigned long ref_clk_rate;
int ret;

- cdns_phy->clk = devm_clk_get(dev, "refclk");
- if (IS_ERR(cdns_phy->clk)) {
- dev_err(dev, "phy ref clock not found\n");
- return PTR_ERR(cdns_phy->clk);
- }
-
ret = clk_prepare_enable(cdns_phy->clk);
if (ret) {
dev_err(cdns_phy->dev, "Failed to prepare ref clock\n");
@@ -2776,6 +2779,10 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
if (ret)
return ret;

+ ret = cdns_torrent_of_get_clk(cdns_phy);
+ if (ret)
+ goto clk_cleanup;
+
regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1, &already_configured);

if (!already_configured) {

--
2.39.2


2024-03-04 16:13:34

by Thomas Richard

[permalink] [raw]
Subject: [PATCH v4 12/18] phy: cadence-torrent: remove noop_ops phy operations

Even if a PHY is already configured, the PHY operations are needed during
resume stage, as the PHY is in reset state.
The noop_ops PHY operations is removed to always have PHY operations.
The already_configured flag is checked at the begening of init, configure
and poweron operations to keep the already_configured behaviour.

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/cadence/phy-cadence-torrent.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
index b35fbc8a60e5..52cadca4c07b 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -1593,6 +1593,9 @@ static int cdns_torrent_dp_configure(struct phy *phy,
struct cdns_torrent_phy *cdns_phy = dev_get_drvdata(phy->dev.parent);
int ret;

+ if (cdns_phy->already_configured)
+ return 0;
+
ret = cdns_torrent_dp_verify_config(inst, &opts->dp);
if (ret) {
dev_err(&phy->dev, "invalid params for phy configure\n");
@@ -1628,6 +1631,12 @@ static int cdns_torrent_phy_on(struct phy *phy)
u32 read_val;
int ret;

+ if (cdns_phy->already_configured) {
+ /* Give 5ms to 10ms delay for the PIPE clock to be stable */
+ usleep_range(5000, 10000);
+ return 0;
+ }
+
if (cdns_phy->nsubnodes == 1) {
/* Take the PHY lane group out of reset */
reset_control_deassert(inst->lnk_rst);
@@ -2306,6 +2315,9 @@ static int cdns_torrent_phy_init(struct phy *phy)
u32 num_regs;
int i, j;

+ if (cdns_phy->already_configured)
+ return 0;
+
if (cdns_phy->nsubnodes > 1) {
if (phy_type == TYPE_DP)
return cdns_torrent_dp_multilink_init(cdns_phy, inst, phy);
@@ -2443,19 +2455,6 @@ static const struct phy_ops cdns_torrent_phy_ops = {
.owner = THIS_MODULE,
};

-static int cdns_torrent_noop_phy_on(struct phy *phy)
-{
- /* Give 5ms to 10ms delay for the PIPE clock to be stable */
- usleep_range(5000, 10000);
-
- return 0;
-}
-
-static const struct phy_ops noop_ops = {
- .power_on = cdns_torrent_noop_phy_on,
- .owner = THIS_MODULE,
-};
-
static
int cdns_torrent_phy_configure_multilink(struct cdns_torrent_phy *cdns_phy)
{
@@ -2869,10 +2868,7 @@ static int cdns_torrent_phy_probe(struct platform_device *pdev)
of_property_read_u32(child, "cdns,ssc-mode",
&cdns_phy->phys[node].ssc_mode);

- if (!cdns_phy->already_configured)
- gphy = devm_phy_create(dev, child, &cdns_torrent_phy_ops);
- else
- gphy = devm_phy_create(dev, child, &noop_ops);
+ gphy = devm_phy_create(dev, child, &cdns_torrent_phy_ops);
if (IS_ERR(gphy)) {
ret = PTR_ERR(gphy);
goto put_child;

--
2.39.2


2024-03-08 10:00:11

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v4 02/18] pinctrl: pinctrl-single: move suspend()/resume() callbacks to noirq

* Thomas Richard <[email protected]> [240304 15:36]:
> The goal is to extend the active period of pinctrl.
> Some devices may need active pinctrl after suspend() and/or before
> resume().

Reviewed-by: Tony Lindgren <[email protected]>

2024-03-08 14:47:31

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v4 03/18] i2c: omap: wakeup the controller during suspend() callback

* Thomas Richard <[email protected]> [240304 15:36]:
> A device may need the controller up during suspend_noirq() or
> resume_noirq().
> But if the controller is autosuspended, there is no way to wakeup it during
> suspend_noirq() or resume_noirq() because runtime pm is disabled at this
> time.
>
> The suspend() callback wakes up the controller, so it is available until
> its suspend_noirq() callback (pm_runtime_force_suspend()).
> During the resume, it's restored by resume_noirq() callback
> (pm_runtime_force_resume()). Then resume() callback enables autosuspend.

Reviewed-by: Tony Lindgren <[email protected]>

2024-03-12 08:03:41

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v4 03/18] i2c: omap: wakeup the controller during suspend() callback

On Fri, Mar 08, 2024 at 10:42:40AM +0200, Tony Lindgren wrote:
> * Thomas Richard <[email protected]> [240304 15:36]:
> > A device may need the controller up during suspend_noirq() or
> > resume_noirq().
> > But if the controller is autosuspended, there is no way to wakeup it during
> > suspend_noirq() or resume_noirq() because runtime pm is disabled at this
> > time.
> >
> > The suspend() callback wakes up the controller, so it is available until
> > its suspend_noirq() callback (pm_runtime_force_suspend()).
> > During the resume, it's restored by resume_noirq() callback
> > (pm_runtime_force_resume()). Then resume() callback enables autosuspend.
>
> Reviewed-by: Tony Lindgren <[email protected]>

I fully trust Tony and I assume that this series should go in via some
other tree than I2C. So:

Acked-by: Wolfram Sang <[email protected]>

Andi, do you agree?


Attachments:
(No filename) (917.00 B)
signature.asc (849.00 B)
Download all attachments

2024-03-13 00:37:59

by Andi Shyti

[permalink] [raw]
Subject: Re: [PATCH v4 03/18] i2c: omap: wakeup the controller during suspend() callback

Hi Thomas,

On Mon, Mar 04, 2024 at 04:35:46PM +0100, Thomas Richard wrote:
> A device may need the controller up during suspend_noirq() or
> resume_noirq().
> But if the controller is autosuspended, there is no way to wakeup it during
> suspend_noirq() or resume_noirq() because runtime pm is disabled at this
> time.
>
> The suspend() callback wakes up the controller, so it is available until
> its suspend_noirq() callback (pm_runtime_force_suspend()).
> During the resume, it's restored by resume_noirq() callback
> (pm_runtime_force_resume()). Then resume() callback enables autosuspend.
>
> So the controller is up during a little time slot in suspend and resume
> sequences even if it's not used.
>
> Signed-off-by: Thomas Richard <[email protected]>

Reviewed-by: Andi Shyti <[email protected]>

Thanks,
Andi

2024-03-13 00:48:37

by Andi Shyti

[permalink] [raw]
Subject: Re: [PATCH v4 03/18] i2c: omap: wakeup the controller during suspend() callback

Hi Wolfram,

On Tue, Mar 12, 2024 at 09:03:27AM +0100, Wolfram Sang wrote:
> On Fri, Mar 08, 2024 at 10:42:40AM +0200, Tony Lindgren wrote:
> > * Thomas Richard <[email protected]> [240304 15:36]:
> > > A device may need the controller up during suspend_noirq() or
> > > resume_noirq().
> > > But if the controller is autosuspended, there is no way to wakeup it during
> > > suspend_noirq() or resume_noirq() because runtime pm is disabled at this
> > > time.
> > >
> > > The suspend() callback wakes up the controller, so it is available until
> > > its suspend_noirq() callback (pm_runtime_force_suspend()).
> > > During the resume, it's restored by resume_noirq() callback
> > > (pm_runtime_force_resume()). Then resume() callback enables autosuspend.
> >
> > Reviewed-by: Tony Lindgren <[email protected]>
>
> I fully trust Tony and I assume that this series should go in via some
> other tree than I2C. So:
>
> Acked-by: Wolfram Sang <[email protected]>
>
> Andi, do you agree?

Agree... but who is going to take this? Eventually I can just
this one.

Andi

2024-03-13 06:21:44

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v4 03/18] i2c: omap: wakeup the controller during suspend() callback

Hi Andi,

> Agree... but who is going to take this? Eventually I can just
> this one.

Usually, it should be stated in the coverletter what the suggested path
for upstreaming is. If it is not, then I apply my gut feeling what
should be better. With this series (and seeing that there are no other
omap patches pending), I assume some other tree so I ack in advance. If
I am wrong and should apply it independently, people will tell me at
this stage. That's how I handle it.

Happy hacking,

Wolfram


Attachments:
(No filename) (519.00 B)
signature.asc (849.00 B)
Download all attachments

2024-03-13 06:52:08

by Siddharth Vadapalli

[permalink] [raw]
Subject: Re: [PATCH v4 18/18] PCI: j721e: Add suspend and resume support

On Mon, Mar 04, 2024 at 04:36:01PM +0100, Thomas Richard wrote:
> From: Théo Lebrun <[email protected]>
>
> Add suspend and resume support. Only the rc mode is supported.
>
> During the suspend stage PERST# is asserted, then deasserted during the
> resume stage.
>
> Signed-off-by: Théo Lebrun <[email protected]>
> Signed-off-by: Thomas Richard <[email protected]>

Reviewed-by: Siddharth Vadapalli <[email protected]>

Regards,
Siddharth.

> ---
> drivers/pci/controller/cadence/pci-j721e.c | 86 ++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 9af4fd64c1f9..a1f1232e8ee5 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -7,6 +7,8 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/container_of.h>
> #include <linux/delay.h>
> #include <linux/gpio/consumer.h>
> #include <linux/io.h>
> @@ -22,6 +24,8 @@
> #include "../../pci.h"
> #include "pcie-cadence.h"
>
> +#define cdns_pcie_to_rc(p) container_of(p, struct cdns_pcie_rc, pcie)
> +
> #define ENABLE_REG_SYS_2 0x108
> #define STATUS_REG_SYS_2 0x508
> #define STATUS_CLR_REG_SYS_2 0x708
> @@ -588,6 +592,87 @@ static void j721e_pcie_remove(struct platform_device *pdev)
> pm_runtime_disable(dev);
> }
>
> +static int j721e_pcie_suspend_noirq(struct device *dev)
> +{
> + struct j721e_pcie *pcie = dev_get_drvdata(dev);
> +
> + if (pcie->mode == PCI_MODE_RC) {
> + gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> + clk_disable_unprepare(pcie->refclk);
> + }
> +
> + cdns_pcie_disable_phy(pcie->cdns_pcie);
> +
> + return 0;
> +}
> +
> +static int j721e_pcie_resume_noirq(struct device *dev)
> +{
> + struct j721e_pcie *pcie = dev_get_drvdata(dev);
> + struct cdns_pcie *cdns_pcie = pcie->cdns_pcie;
> + int ret;
> +
> + ret = j721e_pcie_ctrl_init(pcie);
> + if (ret < 0)
> + return ret;
> +
> + j721e_pcie_config_link_irq(pcie);
> +
> + /*
> + * This is not called explicitly in the probe, it is called by
> + * cdns_pcie_init_phy().
> + */
> + ret = cdns_pcie_enable_phy(pcie->cdns_pcie);
> + if (ret < 0)
> + return ret;
> +
> + if (pcie->mode == PCI_MODE_RC) {
> + struct cdns_pcie_rc *rc = cdns_pcie_to_rc(cdns_pcie);
> +
> + ret = clk_prepare_enable(pcie->refclk);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * "Power Sequencing and Reset Signal Timings" table in
> + * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, REV. 3.0
> + * indicates PERST# should be deasserted after minimum of 100us
> + * once REFCLK is stable. The REFCLK to the connector in RC
> + * mode is selected while enabling the PHY. So deassert PERST#
> + * after 100 us.
> + */
> + if (pcie->reset_gpio) {
> + fsleep(100);
> + gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> + }
> +
> + ret = cdns_pcie_host_link_setup(rc);
> + if (ret < 0) {
> + clk_disable_unprepare(pcie->refclk);
> + return ret;
> + }
> +
> + /*
> + * Reset internal status of BARs to force reinitialization in
> + * cdns_pcie_host_init().
> + */
> + for (enum cdns_pcie_rp_bar bar = RP_BAR0; bar <= RP_NO_BAR; bar++)
> + rc->avail_ib_bar[bar] = true;
> +
> + ret = cdns_pcie_host_init(rc);
> + if (ret) {
> + clk_disable_unprepare(pcie->refclk);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static DEFINE_NOIRQ_DEV_PM_OPS(j721e_pcie_pm_ops,
> + j721e_pcie_suspend_noirq,
> + j721e_pcie_resume_noirq);
> +
> static struct platform_driver j721e_pcie_driver = {
> .probe = j721e_pcie_probe,
> .remove_new = j721e_pcie_remove,
> @@ -595,6 +680,7 @@ static struct platform_driver j721e_pcie_driver = {
> .name = "j721e-pcie",
> .of_match_table = of_j721e_pcie_match,
> .suppress_bind_attrs = true,
> + .pm = pm_sleep_ptr(&j721e_pcie_pm_ops),
> },
> };
> builtin_platform_driver(j721e_pcie_driver);
>
> --
> 2.39.2
>
>

2024-03-20 07:45:34

by Dhruva Gole

[permalink] [raw]
Subject: Re: [PATCH v4 02/18] pinctrl: pinctrl-single: move suspend()/resume() callbacks to noirq

Hi,

On Mar 04, 2024 at 16:35:45 +0100, Thomas Richard wrote:
> The goal is to extend the active period of pinctrl.
> Some devices may need active pinctrl after suspend() and/or before
> resume().
> So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
> have active pinctrl until suspend_noirq() (included), and from
> resume_noirq() (included).
>
> The deprecated API has been removed to use the new one (dev_pm_ops struct).
>
> No need to check the pointer returned by dev_get_drvdata(), as
> platform_set_drvdata() is called during the probe.
>
> Reviewed-by: Linus Walleij <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Thomas Richard <[email protected]>
> ---

I was planning to do this but didn't see particular benefit to it. Do
you see the benefit on your specific device? Can you help me understand
how? Not against the patch, just curious.

Reviewed-by: Dhruva Gole <[email protected]>

--
Best regards,
Dhruva

2024-03-20 08:37:46

by Thomas Richard

[permalink] [raw]
Subject: Re: [PATCH v4 02/18] pinctrl: pinctrl-single: move suspend()/resume() callbacks to noirq

On 3/20/24 08:44, Dhruva Gole wrote:
> Hi,
>
> On Mar 04, 2024 at 16:35:45 +0100, Thomas Richard wrote:
>> The goal is to extend the active period of pinctrl.
>> Some devices may need active pinctrl after suspend() and/or before
>> resume().
>> So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
>> have active pinctrl until suspend_noirq() (included), and from
>> resume_noirq() (included).
>>
>> The deprecated API has been removed to use the new one (dev_pm_ops struct).
>>
>> No need to check the pointer returned by dev_get_drvdata(), as
>> platform_set_drvdata() is called during the probe.
>>
>> Reviewed-by: Linus Walleij <[email protected]>
>> Reviewed-by: Andy Shevchenko <[email protected]>
>> Signed-off-by: Thomas Richard <[email protected]>
>> ---
>
> I was planning to do this but didn't see particular benefit to it. Do
> you see the benefit on your specific device? Can you help me understand
> how? Not against the patch, just curious.

Hello Dhruva,

We need this patch to support suspend to ram for the PCIe on J7200.
In root complex mode, a gpio is used to reset endpoints.
This gpio shall be managed during suspend_noirq and resume_noirq stages.
On J7200 this gpio is on a gpio expander.
So we need this patch to restore pinctrl to be able to do i2c accesses
in noirq stages.

Best Regards,

--
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


2024-03-28 21:14:17

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v4 02/18] pinctrl: pinctrl-single: move suspend()/resume() callbacks to noirq

On Mon, Mar 4, 2024 at 4:36 PM Thomas Richard
<[email protected]> wrote:


> The goal is to extend the active period of pinctrl.
> Some devices may need active pinctrl after suspend() and/or before
> resume().
> So move suspend()/resume() to suspend_noirq()/resume_noirq() in order to
> have active pinctrl until suspend_noirq() (included), and from
> resume_noirq() (included).
>
> The deprecated API has been removed to use the new one (dev_pm_ops struct).
>
> No need to check the pointer returned by dev_get_drvdata(), as
> platform_set_drvdata() is called during the probe.
>
> Reviewed-by: Linus Walleij <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Thomas Richard <[email protected]>

Since this patch looks independent from the rest I ripped it out of the
patch series and applied it to the pinctrl tree for kernel v6.10.

Yours,
Linus Walleij

2024-03-28 22:17:10

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 18/18] PCI: j721e: Add suspend and resume support

On Mon, Mar 04, 2024 at 04:36:01PM +0100, Thomas Richard wrote:
> From: Théo Lebrun <[email protected]>
>
> Add suspend and resume support. Only the rc mode is supported.
>
> During the suspend stage PERST# is asserted, then deasserted during the
> resume stage.

> + * "Power Sequencing and Reset Signal Timings" table in
> + * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, REV. 3.0
> + * indicates PERST# should be deasserted after minimum of 100us
> + * once REFCLK is stable. The REFCLK to the connector in RC
> + * mode is selected while enabling the PHY. So deassert PERST#
> + * after 100 us.

Please cite current spec (r5.1 was published August 2023), section,
and parameter name. I think this is T_PERST-CLK, "REFCLK stable
before PERST# inactive", from sec 2.9.2.

> + */
> + if (pcie->reset_gpio) {
> + fsleep(100);

I'd like to see a macro used here instead of a bare number. Since
this isn't anything specific to j721e, maybe add something like
#define PCIE_T_PERST_CLK_US alongside PCIE_T_PVPERL_MS.

> + gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> + }

2024-04-05 16:43:17

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v4 07/18] phy: ti: phy-j721e-wiz: split wiz_clock_init() function

On 04-03-24, 16:35, Thomas Richard wrote:
> The wiz_clock_init() function mixes probe and hardware configuration.
> Rename the wiz_clock_init() to wiz_clock_probe() and move the hardware
> configuration part in a new function named wiz_clock_init().
>
> This hardware configuration sequence must be called during the resume
> stage of the driver.

Do you have phy patches dependent on rest, if not consider submitting
them in a separate series

>
> Signed-off-by: Thomas Richard <[email protected]>
> ---
> drivers/phy/ti/phy-j721e-wiz.c | 67 ++++++++++++++++++++++++------------------
> 1 file changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index 5fea4df9404e..0e3cb1ed5a52 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -1076,26 +1076,12 @@ static int wiz_clock_register(struct wiz *wiz)
> return ret;
> }
>
> -static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> +static void wiz_clock_init(struct wiz *wiz)
> {
> - const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
> - struct device *dev = wiz->dev;
> - struct device_node *clk_node;
> - const char *node_name;
> unsigned long rate;
> - struct clk *clk;
> - int ret;
> - int i;
> -
> - clk = devm_clk_get(dev, "core_ref_clk");
> - if (IS_ERR(clk))
> - return dev_err_probe(dev, PTR_ERR(clk),
> - "core_ref_clk clock not found\n");
>
> - wiz->input_clks[WIZ_CORE_REFCLK] = clk;
> -
> - rate = clk_get_rate(clk);
> - if (rate >= 100000000)
> + rate = clk_get_rate(wiz->input_clks[WIZ_CORE_REFCLK]);
> + if (rate >= REF_CLK_100MHZ)
> regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x1);
> else
> regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x3);
> @@ -1119,6 +1105,39 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> break;
> }
>
> + if (wiz->input_clks[WIZ_CORE_REFCLK1]) {
> + rate = clk_get_rate(wiz->input_clks[WIZ_CORE_REFCLK1]);
> + if (rate >= REF_CLK_100MHZ)
> + regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
> + else
> + regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
> +

unnecessary empty line

> + }
> +
> + rate = clk_get_rate(wiz->input_clks[WIZ_EXT_REFCLK]);
> + if (rate >= REF_CLK_100MHZ)
> + regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
> + else
> + regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
> +}
> +
> +static int wiz_clock_probe(struct wiz *wiz, struct device_node *node)
> +{
> + const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
> + struct device *dev = wiz->dev;
> + struct device_node *clk_node;
> + const char *node_name;
> + struct clk *clk;
> + int ret;
> + int i;
> +
> + clk = devm_clk_get(dev, "core_ref_clk");
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk),
> + "core_ref_clk clock not found\n");
> +
> + wiz->input_clks[WIZ_CORE_REFCLK] = clk;
> +
> if (wiz->data->pma_cmn_refclk1_int_mode) {
> clk = devm_clk_get(dev, "core_ref1_clk");
> if (IS_ERR(clk))
> @@ -1126,12 +1145,6 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> "core_ref1_clk clock not found\n");
>
> wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
> -
> - rate = clk_get_rate(clk);
> - if (rate >= 100000000)
> - regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
> - else
> - regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
> }
>
> clk = devm_clk_get(dev, "ext_ref_clk");
> @@ -1141,11 +1154,7 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
>
> wiz->input_clks[WIZ_EXT_REFCLK] = clk;
>
> - rate = clk_get_rate(clk);
> - if (rate >= 100000000)
> - regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
> - else
> - regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
> + wiz_clock_init(wiz);
>
> switch (wiz->type) {
> case AM64_WIZ_10G:
> @@ -1589,7 +1598,7 @@ static int wiz_probe(struct platform_device *pdev)
> goto err_get_sync;
> }
>
> - ret = wiz_clock_init(wiz, node);
> + ret = wiz_clock_probe(wiz, node);
> if (ret < 0) {
> dev_warn(dev, "Failed to initialize clocks\n");
> goto err_get_sync;
>
> --
> 2.39.2

--
~Vinod

2024-04-05 16:44:41

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v4 08/18] phy: ti: phy-j721e-wiz: add resume support

On 04-03-24, 16:35, Thomas Richard wrote:
> Add resume support.
> It has been tested on J7200 SR1.0 and SR2.0.
>
> Co-developed-by: Th?o Lebrun <[email protected]>
> Signed-off-by: Th?o Lebrun <[email protected]>
> Signed-off-by: Thomas Richard <[email protected]>
> ---
> drivers/phy/ti/phy-j721e-wiz.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index 0e3cb1ed5a52..b2320f2efb72 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -1660,12 +1660,41 @@ static void wiz_remove(struct platform_device *pdev)
> pm_runtime_disable(dev);
> }
>
> +static int wiz_resume_noirq(struct device *dev)

I think this should be annotated with __maybe_unused

> +{
> + struct device_node *node = dev->of_node;
> + struct wiz *wiz = dev_get_drvdata(dev);
> + int ret;
> +
> + /* Enable supplemental Control override if available */
> + if (wiz->sup_legacy_clk_override)
> + regmap_field_write(wiz->sup_legacy_clk_override, 1);
> +
> + wiz_clock_init(wiz);
> +
> + ret = wiz_init(wiz);
> + if (ret) {
> + dev_err(dev, "WIZ initialization failed\n");
> + goto err_wiz_init;
> + }
> +
> + return 0;
> +
> +err_wiz_init:
> + wiz_clock_cleanup(wiz, node);
> +
> + return ret;
> +}
> +
> +static DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, NULL, wiz_resume_noirq);
> +
> static struct platform_driver wiz_driver = {
> .probe = wiz_probe,
> .remove_new = wiz_remove,
> .driver = {
> .name = "wiz",
> .of_match_table = wiz_id_table,
> + .pm = pm_sleep_ptr(&wiz_pm_ops),
> },
> };
> module_platform_driver(wiz_driver);
>
> --
> 2.39.2

--
~Vinod

2024-04-05 17:26:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 08/18] phy: ti: phy-j721e-wiz: add resume support

On Fri, Apr 05, 2024 at 10:14:09PM +0530, Vinod Koul wrote:
> On 04-03-24, 16:35, Thomas Richard wrote:

..

> > +static int wiz_resume_noirq(struct device *dev)
>
> I think this should be annotated with __maybe_unused

No...

> > + .pm = pm_sleep_ptr(&wiz_pm_ops),

..because of magic of PTR_IF() here.

--
With Best Regards,
Andy Shevchenko



2024-04-10 07:52:16

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH v4 16/18] PCI: j721e: Use dev_err_probe() in the probe() function

On Mon, Mar 04, 2024 at 04:35:59PM +0100, Thomas Richard wrote:
> Use dev_err_probe() instead of dev_err() in the probe() function to
> simplify the code and standardize the error output.
>
> Signed-off-by: Thomas Richard <[email protected]>

Reviewed-by: Francesco Dolcini <[email protected]>

Franceco