2022-02-16 06:31:53

by Jon Lin

[permalink] [raw]
Subject: [PATCH v3 0/6] New support and problem adjustment of SPI rockchip



Changes in v3:
- Support clear the bits of configure bits filed

Changes in v2:
- Fix patches should be at the start of the series
- Fix patches should be at the start of the series
- Delete useless messages
- Limit cs-high presetting to the chip select n <= 1

Changes in v1:
- The origin patch

Jon Lin (5):
spi: rockchip: Fix error in getting num-cs property
spi: rockchip: terminate dma transmission when slave abort
spi: rockchip: Stop spi slave dma receiver when cs inactive
spi: rockchip: Preset cs-high and clk polarity in setup progress
spi: rockchip: clear interrupt status in error handler

shengfei Xu (1):
spi: rockchip: Suspend and resume the bus during NOIRQ_SYSTEM_SLEEP_PM
ops

drivers/spi/spi-rockchip.c | 135 ++++++++++++++++++++++++++++++++-----
1 file changed, 118 insertions(+), 17 deletions(-)

--
2.17.1


2022-02-16 06:33:10

by Jon Lin

[permalink] [raw]
Subject: [PATCH v3 5/6] spi: rockchip: Suspend and resume the bus during NOIRQ_SYSTEM_SLEEP_PM ops

From: shengfei Xu <[email protected]>

the wakeup interrupt handler which is guaranteed not to run while
@resume noirq() is being executed. the patch can help to avoid the
wakeup source try to access spi when the spi is in suspend mode.

Signed-off-by: shengfei Xu <[email protected]>
Signed-off-by: Jon Lin <[email protected]>
---

Changes in v3: None
Changes in v2: None
Changes in v1: None

drivers/spi/spi-rockchip.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 83da8fdb3c02..8b4d56ee2193 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -964,14 +964,14 @@ static int rockchip_spi_suspend(struct device *dev)
{
int ret;
struct spi_controller *ctlr = dev_get_drvdata(dev);
+ struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);

ret = spi_controller_suspend(ctlr);
if (ret < 0)
return ret;

- ret = pm_runtime_force_suspend(dev);
- if (ret < 0)
- return ret;
+ clk_disable_unprepare(rs->spiclk);
+ clk_disable_unprepare(rs->apb_pclk);

pinctrl_pm_select_sleep_state(dev);

@@ -986,10 +986,14 @@ static int rockchip_spi_resume(struct device *dev)

pinctrl_pm_select_default_state(dev);

- ret = pm_runtime_force_resume(dev);
+ ret = clk_prepare_enable(rs->apb_pclk);
if (ret < 0)
return ret;

+ ret = clk_prepare_enable(rs->spiclk);
+ if (ret < 0)
+ clk_disable_unprepare(rs->apb_pclk);
+
ret = spi_controller_resume(ctlr);
if (ret < 0) {
clk_disable_unprepare(rs->spiclk);
@@ -1031,7 +1035,7 @@ static int rockchip_spi_runtime_resume(struct device *dev)
#endif /* CONFIG_PM */

static const struct dev_pm_ops rockchip_spi_pm = {
- SET_SYSTEM_SLEEP_PM_OPS(rockchip_spi_suspend, rockchip_spi_resume)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_spi_suspend, rockchip_spi_resume)
SET_RUNTIME_PM_OPS(rockchip_spi_runtime_suspend,
rockchip_spi_runtime_resume, NULL)
};
--
2.17.1

2022-02-16 07:28:35

by Jon Lin

[permalink] [raw]
Subject: [PATCH v3 2/6] spi: rockchip: terminate dma transmission when slave abort

After slave abort, all DMA should be stopped, or it will affect the
next transmission and maybe abort again.

Signed-off-by: Jon Lin <[email protected]>
---

Changes in v3: None
Changes in v2:
- Fix patches should be at the start of the series
- Delete useless messages

Changes in v1: None

drivers/spi/spi-rockchip.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 4f65ba3dd19c..c6a1bb09be05 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -585,6 +585,12 @@ static int rockchip_spi_slave_abort(struct spi_controller *ctlr)
{
struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);

+ if (atomic_read(&rs->state) & RXDMA)
+ dmaengine_terminate_sync(ctlr->dma_rx);
+ if (atomic_read(&rs->state) & TXDMA)
+ dmaengine_terminate_sync(ctlr->dma_tx);
+ atomic_set(&rs->state, 0);
+ spi_enable_chip(rs, false);
rs->slave_abort = true;
spi_finalize_current_transfer(ctlr);

--
2.17.1

2022-02-17 17:37:49

by Mark Brown

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/6] New support and problem adjustment of SPI rockchip

On Wed, 16 Feb 2022 09:40:22 +0800, Jon Lin wrote:
> Changes in v3:
> - Support clear the bits of configure bits filed
>
> Changes in v2:
> - Fix patches should be at the start of the series
> - Fix patches should be at the start of the series
> - Delete useless messages
> - Limit cs-high presetting to the chip select n <= 1
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-linus

Thanks!

[1/6] spi: rockchip: Fix error in getting num-cs property
commit: 9382df0a98aad5bbcd4d634790305a1d786ad224
[2/6] spi: rockchip: terminate dma transmission when slave abort
commit: 80808768e41324d2e23de89972b5406c1020e6e4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2022-02-18 00:07:22

by Mark Brown

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/6] New support and problem adjustment of SPI rockchip

On Wed, 16 Feb 2022 09:40:22 +0800, Jon Lin wrote:
> Changes in v3:
> - Support clear the bits of configure bits filed
>
> Changes in v2:
> - Fix patches should be at the start of the series
> - Fix patches should be at the start of the series
> - Delete useless messages
> - Limit cs-high presetting to the chip select n <= 1
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[3/6] spi: rockchip: Stop spi slave dma receiver when cs inactive
commit: 869f2c94db92f0f1d6acd0dff1c1ebb8160f5e29
[4/6] spi: rockchip: Preset cs-high and clk polarity in setup progress
commit: 3a4bf922d42efa4e9a3dc803d1fd786d43e8a501
[5/6] spi: rockchip: Suspend and resume the bus during NOIRQ_SYSTEM_SLEEP_PM ops
commit: e882575efc771f130a24322377dc1033551da11d
[6/6] spi: rockchip: clear interrupt status in error handler
commit: 2fcdde56c44fe1cd13ce328128f509bbda2cdb41

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2022-06-21 15:54:40

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] spi: rockchip: Suspend and resume the bus during NOIRQ_SYSTEM_SLEEP_PM ops

Hello Jon,

On Wed, Feb 16, 2022 at 09:40:27AM +0800, Jon Lin wrote:
> From: shengfei Xu <[email protected]>
>
> the wakeup interrupt handler which is guaranteed not to run while
> @resume noirq() is being executed. the patch can help to avoid the
> wakeup source try to access spi when the spi is in suspend mode.

This patch causes oops on suspend every single time, because it tries to disable
already disabled clocks (one disable in runtime PM suspend, and other one in
system suspend). It also fails to properly handle errors from clk_prepare_enable
in rockchip_spi_resume, potentially causing even more clock enable/disable
imballance issues.

Please send a revert and figure out a better fix for the original issue.

kind regards,
Ondrej

------------[ cut here ]------------
clk_spi1 already unprepared
WARNING: CPU: 5 PID: 423 at drivers/clk/clk.c:829 clk_core_unprepare+0x1a0/0x1c0
Modules linked in:
CPU: 5 PID: 423 Comm: waketest-onesho Tainted: G W 5.18.5-00013-g12be33cc5bf1-dirty #142
Hardware name: Pine64 PinePhonePro (DT)
pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clk_core_unprepare+0x1a0/0x1c0
lr : clk_core_unprepare+0x1a0/0x1c0
sp : ffff80000acf3a30
x29: ffff80000acf3a30 x28: 0000000000000000 x27: 0000000bbb2b9e2b
x26: 0000000000000002 x25: ffff8000088310b0 x24: 0000000000000000
x23: ffff800009fe2000 x22: 0000000000000002 x21: ffff0000044fe980
x20: ffff0000042d2600 x19: ffff00000269da00 x18: ffffffffffffffff
x17: 000000040044ffff x16: 00000034b5503510 x15: 0000000000000000
x14: ffff0000027b0d80 x13: 0a64657261706572 x12: 706e752079646165
x11: 0000000000000001 x10: fffffffffffe0000 x9 : ffff800008176438
x8 : ffff80000acf3a30 x7 : 61706572706e7520 x6 : 0000000000000001
x5 : ffff0000f77a4790 x4 : 0000000000000000 x3 : 0000000000000027
x2 : 0000000000000000 x1 : 0000000000000027 x0 : 000000000000001b
Call trace:
clk_core_unprepare+0x1a0/0x1c0
clk_unprepare+0x30/0x44
rockchip_spi_suspend+0x40/0x70
dpm_run_callback+0x5c/0x170
__device_suspend_noirq+0xc8/0x2a0
dpm_noirq_suspend_devices+0x118/0x2a0
dpm_suspend_noirq+0x28/0x94
suspend_devices_and_enter+0x184/0x840
pm_suspend+0x374/0x470
state_store+0x84/0x100
kobj_attr_store+0x14/0x2c
sysfs_kf_write+0x48/0x54
kernfs_fop_write_iter+0x120/0x1e4
new_sync_write+0xc0/0x12c
vfs_write+0x210/0x2d0
ksys_write+0x60/0xe0
__arm64_sys_write+0x20/0x30
invoke_syscall+0x4c/0x110
el0_svc_common.constprop.0+0x48/0xf0
do_el0_svc+0x28/0x80
el0_svc+0x14/0x50
el0t_64_sync_handler+0xe0/0x110
el0t_64_sync+0x148/0x14c
---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
pclk_spi1 already disabled
WARNING: CPU: 5 PID: 423 at drivers/clk/clk.c:971 clk_core_disable+0x1d8/0x1f0
Modules linked in:
CPU: 5 PID: 423 Comm: waketest-onesho Tainted: G W 5.18.5-00013-g12be33cc5bf1-dirty #142
Hardware name: Pine64 PinePhonePro (DT)
pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clk_core_disable+0x1d8/0x1f0
lr : clk_core_disable+0x1d8/0x1f0
sp : ffff80000acf3a20
x29: ffff80000acf3a20 x28: 0000000000000000 x27: 0000000bbb2b9e2b
x26: 0000000000000002 x25: ffff8000088310b0 x24: 0000000000000000
x23: ffff800009fe2000 x22: 0000000000000002 x21: ffff0000044fe980
x20: ffff000002687a00 x19: ffff000002687a00 x18: ffffffffffffffff
x17: 000000040044ffff x16: 00000034b5503510 x15: 0000000000000000
x14: ffff0000027b0d80 x13: 0a64656c62617369 x12: 642079646165726c
x11: 0000000000000001 x10: fffffffffffe0000 x9 : ffff800008176438
x8 : ffff80000acf3a20 x7 : 656c626173696420 x6 : 0000000000000001
x5 : ffff0000f77a4790 x4 : 0000000000000000 x3 : 0000000000000027
x2 : 0000000000000000 x1 : 0000000000000027 x0 : 000000000000001a
Call trace:
clk_core_disable+0x1d8/0x1f0
clk_disable+0x34/0x4c
rockchip_spi_suspend+0x4c/0x70
dpm_run_callback+0x5c/0x170
__device_suspend_noirq+0xc8/0x2a0
dpm_noirq_suspend_devices+0x118/0x2a0
dpm_suspend_noirq+0x28/0x94
suspend_devices_and_enter+0x184/0x840
pm_suspend+0x374/0x470
state_store+0x84/0x100
kobj_attr_store+0x14/0x2c
sysfs_kf_write+0x48/0x54
kernfs_fop_write_iter+0x120/0x1e4
new_sync_write+0xc0/0x12c
vfs_write+0x210/0x2d0
ksys_write+0x60/0xe0
__arm64_sys_write+0x20/0x30
invoke_syscall+0x4c/0x110
el0_svc_common.constprop.0+0x48/0xf0
do_el0_svc+0x28/0x80
el0_svc+0x14/0x50
el0t_64_sync_handler+0xe0/0x110
el0t_64_sync+0x148/0x14c
---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
pclk_spi1 already unprepared
WARNING: CPU: 5 PID: 423 at drivers/clk/clk.c:829 clk_core_unprepare+0x1a0/0x1c0
Modules linked in:
CPU: 5 PID: 423 Comm: waketest-onesho Tainted: G W 5.18.5-00013-g12be33cc5bf1-dirty #142
Hardware name: Pine64 PinePhonePro (DT)
pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clk_core_unprepare+0x1a0/0x1c0
lr : clk_core_unprepare+0x1a0/0x1c0
sp : ffff80000acf3a30
x29: ffff80000acf3a30 x28: 0000000000000000 x27: 0000000bbb2b9e2b
x26: 0000000000000002 x25: ffff8000088310b0 x24: 0000000000000000
x23: ffff800009fe2000 x22: 0000000000000002 x21: ffff0000044fe980
x20: ffff0000044fe900 x19: ffff000002687a00 x18: ffffffffffffffff
x17: 000000040044ffff x16: 00000034b5503510 x15: 0000000000000000
x14: ffff0000027b0d80 x13: 0a64657261706572 x12: 706e752079646165
x11: 0000000000000001 x10: fffffffffffe0000 x9 : ffff800008176438
x8 : ffff80000acf3a30 x7 : 61706572706e7520 x6 : 0000000000000001
x5 : ffff0000f77a4790 x4 : 0000000000000000 x3 : 0000000000000027
x2 : 0000000000000000 x1 : 0000000000000027 x0 : 000000000000001c
Call trace:
clk_core_unprepare+0x1a0/0x1c0
clk_unprepare+0x30/0x44
rockchip_spi_suspend+0x54/0x70
dpm_run_callback+0x5c/0x170
__device_suspend_noirq+0xc8/0x2a0
dpm_noirq_suspend_devices+0x118/0x2a0
dpm_suspend_noirq+0x28/0x94
suspend_devices_and_enter+0x184/0x840
pm_suspend+0x374/0x470
state_store+0x84/0x100
kobj_attr_store+0x14/0x2c
sysfs_kf_write+0x48/0x54
kernfs_fop_write_iter+0x120/0x1e4
new_sync_write+0xc0/0x12c
vfs_write+0x210/0x2d0
ksys_write+0x60/0xe0
__arm64_sys_write+0x20/0x30
invoke_syscall+0x4c/0x110
el0_svc_common.constprop.0+0x48/0xf0
do_el0_svc+0x28/0x80
el0_svc+0x14/0x50
el0t_64_sync_handler+0xe0/0x110
el0t_64_sync+0x148/0x14c
---[ end trace 0000000000000000 ]---

> Signed-off-by: shengfei Xu <[email protected]>
> Signed-off-by: Jon Lin <[email protected]>
> ---
>
> Changes in v3: None
> Changes in v2: None
> Changes in v1: None
>
> drivers/spi/spi-rockchip.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 83da8fdb3c02..8b4d56ee2193 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -964,14 +964,14 @@ static int rockchip_spi_suspend(struct device *dev)
> {
> int ret;
> struct spi_controller *ctlr = dev_get_drvdata(dev);
> + struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);
>
> ret = spi_controller_suspend(ctlr);
> if (ret < 0)
> return ret;
>
> - ret = pm_runtime_force_suspend(dev);
> - if (ret < 0)
> - return ret;
> + clk_disable_unprepare(rs->spiclk);
> + clk_disable_unprepare(rs->apb_pclk);
>
> pinctrl_pm_select_sleep_state(dev);
>
> @@ -986,10 +986,14 @@ static int rockchip_spi_resume(struct device *dev)
>
> pinctrl_pm_select_default_state(dev);
>
> - ret = pm_runtime_force_resume(dev);
> + ret = clk_prepare_enable(rs->apb_pclk);
> if (ret < 0)
> return ret;
>
> + ret = clk_prepare_enable(rs->spiclk);
> + if (ret < 0)
> + clk_disable_unprepare(rs->apb_pclk);
> +
> ret = spi_controller_resume(ctlr);
> if (ret < 0) {
> clk_disable_unprepare(rs->spiclk);
> @@ -1031,7 +1035,7 @@ static int rockchip_spi_runtime_resume(struct device *dev)
> #endif /* CONFIG_PM */
>
> static const struct dev_pm_ops rockchip_spi_pm = {
> - SET_SYSTEM_SLEEP_PM_OPS(rockchip_spi_suspend, rockchip_spi_resume)
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_spi_suspend, rockchip_spi_resume)
> SET_RUNTIME_PM_OPS(rockchip_spi_runtime_suspend,
> rockchip_spi_runtime_resume, NULL)
> };
> --
> 2.17.1
>
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip