[PATCH 1/2] remoteproc: qcom_q6v5_pas: disable wakeup on probe fail or remove

[PATCH 1/2] remoteproc: qcom_q6v5_pas: disable wakeup on probe fail or remove

[Luca Weiss]

Leaving wakeup enabled during probe fail (-EPROBE_DEFER) or remove makes
the subsequent probe fail.

[    3.749454] remoteproc remoteproc0: releasing 3000000.remoteproc
[    3.752949] qcom_q6v5_pas: probe of 3000000.remoteproc failed with error -17
[    3.878935] remoteproc remoteproc0: releasing 4080000.remoteproc
[    3.887602] qcom_q6v5_pas: probe of 4080000.remoteproc failed with error -17
[    4.319552] remoteproc remoteproc0: releasing 8300000.remoteproc
[    4.332716] qcom_q6v5_pas: probe of 8300000.remoteproc failed with error -17

Fix this by disabling wakeup in both cases so the driver can properly
probe on the next try.

Fixes: a781e5aa5911 ("remoteproc: core: Prevent system suspend during remoteproc recovery")
Fixes: dc86c129b4fb ("remoteproc: qcom: pas: Mark devices as wakeup capable")
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 6afd0941e552..d830bf13c32c 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -557,6 +557,7 @@ static int adsp_probe(struct platform_device *pdev)
 	adsp_pds_detach(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
 free_rproc:
 	rproc_free(rproc);
+	device_init_wakeup(adsp->dev, false);
 
 	return ret;
 }
@@ -573,6 +574,7 @@ static int adsp_remove(struct platform_device *pdev)
 	qcom_remove_smd_subdev(adsp->rproc, &adsp->smd_subdev);
 	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
 	rproc_free(adsp->rproc);
+	device_init_wakeup(adsp->dev, false);
 
 	return 0;
 }
-- 
2.38.1

[PATCH 2/2] remoteproc: qcom_q6v5_pas: detach power domains on remove

[Luca Weiss]

We need to detach from the power domains also on remove, not just on
probe fail so a subsequent probe works as expected.

Otherwise the following error appears on re-probe:

[   29.452005] sysfs: cannot create duplicate filename '/devices/genpd:0:3000000.remoteproc'
[   29.477121] CPU: 1 PID: 483 Comm: sh Tainted: G        W          6.1.0-rc4-00075-g71a113770bda #78
[   29.510319] Hardware name: Fairphone 4 (DT)
[   29.538335] Call trace:
[   29.564470]  dump_backtrace.part.0+0xe0/0xf0
[   29.592602]  show_stack+0x18/0x30
[   29.619616]  dump_stack_lvl+0x64/0x80
[   29.646834]  dump_stack+0x18/0x34
[   29.673541]  sysfs_warn_dup+0x60/0x7c
[   29.700592]  sysfs_create_dir_ns+0xec/0x110
[   29.728057]  kobject_add_internal+0xb8/0x374
[   29.755530]  kobject_add+0x9c/0x104
[   29.782072]  device_add+0xbc/0x8a0
[   29.808445]  device_register+0x20/0x30
[   29.835175]  genpd_dev_pm_attach_by_id+0xa4/0x190
[   29.862851]  genpd_dev_pm_attach_by_name+0x3c/0xb0
[   29.890472]  dev_pm_domain_attach_by_name+0x20/0x30
[   29.918212]  adsp_probe+0x278/0x580
[   29.944384]  platform_probe+0x68/0xc0
[   29.970603]  really_probe+0xbc/0x2dc
[   29.996662]  __driver_probe_device+0x78/0xe0
[   30.023491]  device_driver_attach+0x48/0xac
[   30.050215]  bind_store+0xb8/0x114
[   30.075957]  drv_attr_store+0x24/0x3c
[   30.101874]  sysfs_kf_write+0x44/0x54
[   30.127751]  kernfs_fop_write_iter+0x120/0x1f0
[   30.154448]  vfs_write+0x1ac/0x380
[   30.179937]  ksys_write+0x70/0x104
[   30.205274]  __arm64_sys_write+0x1c/0x2c
[   30.231060]  invoke_syscall+0x48/0x114
[   30.256594]  el0_svc_common.constprop.0+0x44/0xec
[   30.283183]  do_el0_svc+0x2c/0xd0
[   30.308320]  el0_svc+0x2c/0x84
[   30.333059]  el0t_64_sync_handler+0xf4/0x120
[   30.359001]  el0t_64_sync+0x18c/0x190
[   30.384385] kobject_add_internal failed for genpd:0:3000000.remoteproc with -EEXIST, don't try to register things with the same name in the same directory.
[   30.406029] remoteproc remoteproc0: releasing 3000000.remoteproc
[   30.416064] qcom_q6v5_pas: probe of 3000000.remoteproc failed with error -17

Fixes: 17ee2fb4e856 ("remoteproc: qcom: pas: Vote for active/proxy power domains")
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index d830bf13c32c..747155c85085 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -573,6 +573,7 @@ static int adsp_remove(struct platform_device *pdev)
 	qcom_remove_sysmon_subdev(adsp->sysmon);
 	qcom_remove_smd_subdev(adsp->rproc, &adsp->smd_subdev);
 	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
+	adsp_pds_detach(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
 	rproc_free(adsp->rproc);
 	device_init_wakeup(adsp->dev, false);
 
-- 
2.38.1

Re: [PATCH 2/2] remoteproc: qcom_q6v5_pas: detach power domains on remove

[Sibi Sankar]

Hey Luca,

Thanks for the fix.


On 11/11/22 17:24, Luca Weiss wrote:
> We need to detach from the power domains also on remove, not just on
> probe fail so a subsequent probe works as expected.
> 
> Otherwise the following error appears on re-probe:
> 
> [   29.452005] sysfs: cannot create duplicate filename '/devices/genpd:0:3000000.remoteproc'
> [   29.477121] CPU: 1 PID: 483 Comm: sh Tainted: G        W          6.1.0-rc4-00075-g71a113770bda #78
> [   29.510319] Hardware name: Fairphone 4 (DT)
> [   29.538335] Call trace:
> [   29.564470]  dump_backtrace.part.0+0xe0/0xf0
> [   29.592602]  show_stack+0x18/0x30
> [   29.619616]  dump_stack_lvl+0x64/0x80
> [   29.646834]  dump_stack+0x18/0x34
> [   29.673541]  sysfs_warn_dup+0x60/0x7c
> [   29.700592]  sysfs_create_dir_ns+0xec/0x110
> [   29.728057]  kobject_add_internal+0xb8/0x374
> [   29.755530]  kobject_add+0x9c/0x104
> [   29.782072]  device_add+0xbc/0x8a0
> [   29.808445]  device_register+0x20/0x30
> [   29.835175]  genpd_dev_pm_attach_by_id+0xa4/0x190
> [   29.862851]  genpd_dev_pm_attach_by_name+0x3c/0xb0
> [   29.890472]  dev_pm_domain_attach_by_name+0x20/0x30
> [   29.918212]  adsp_probe+0x278/0x580
> [   29.944384]  platform_probe+0x68/0xc0
> [   29.970603]  really_probe+0xbc/0x2dc
> [   29.996662]  __driver_probe_device+0x78/0xe0
> [   30.023491]  device_driver_attach+0x48/0xac
> [   30.050215]  bind_store+0xb8/0x114
> [   30.075957]  drv_attr_store+0x24/0x3c
> [   30.101874]  sysfs_kf_write+0x44/0x54
> [   30.127751]  kernfs_fop_write_iter+0x120/0x1f0
> [   30.154448]  vfs_write+0x1ac/0x380
> [   30.179937]  ksys_write+0x70/0x104
> [   30.205274]  __arm64_sys_write+0x1c/0x2c
> [   30.231060]  invoke_syscall+0x48/0x114
> [   30.256594]  el0_svc_common.constprop.0+0x44/0xec
> [   30.283183]  do_el0_svc+0x2c/0xd0
> [   30.308320]  el0_svc+0x2c/0x84
> [   30.333059]  el0t_64_sync_handler+0xf4/0x120
> [   30.359001]  el0t_64_sync+0x18c/0x190
> [   30.384385] kobject_add_internal failed for genpd:0:3000000.remoteproc with -EEXIST, don't try to register things with the same name in the same directory.
> [   30.406029] remoteproc remoteproc0: releasing 3000000.remoteproc
> [   30.416064] qcom_q6v5_pas: probe of 3000000.remoteproc failed with error -17
> 
> Fixes: 17ee2fb4e856 ("remoteproc: qcom: pas: Vote for active/proxy power domains")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>

> ---
>   drivers/remoteproc/qcom_q6v5_pas.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index d830bf13c32c..747155c85085 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -573,6 +573,7 @@ static int adsp_remove(struct platform_device *pdev)
>   	qcom_remove_sysmon_subdev(adsp->sysmon);
>   	qcom_remove_smd_subdev(adsp->rproc, &adsp->smd_subdev);
>   	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
> +	adsp_pds_detach(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>   	rproc_free(adsp->rproc);
>   	device_init_wakeup(adsp->dev, false);
>   

Re: [PATCH 1/2] remoteproc: qcom_q6v5_pas: disable wakeup on probe fail or remove

[Sibi Sankar]

Hey Luca,

Thanks for the patch.

On 11/11/22 17:24, Luca Weiss wrote:
> Leaving wakeup enabled during probe fail (-EPROBE_DEFER) or remove makes
> the subsequent probe fail.
> 
> [    3.749454] remoteproc remoteproc0: releasing 3000000.remoteproc
> [    3.752949] qcom_q6v5_pas: probe of 3000000.remoteproc failed with error -17
> [    3.878935] remoteproc remoteproc0: releasing 4080000.remoteproc
> [    3.887602] qcom_q6v5_pas: probe of 4080000.remoteproc failed with error -17
> [    4.319552] remoteproc remoteproc0: releasing 8300000.remoteproc
> [    4.332716] qcom_q6v5_pas: probe of 8300000.remoteproc failed with error -17
> 
> Fix this by disabling wakeup in both cases so the driver can properly
> probe on the next try.
> 
> Fixes: a781e5aa5911 ("remoteproc: core: Prevent system suspend during remoteproc recovery")
> Fixes: dc86c129b4fb ("remoteproc: qcom: pas: Mark devices as wakeup capable")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>   drivers/remoteproc/qcom_q6v5_pas.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 6afd0941e552..d830bf13c32c 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -557,6 +557,7 @@ static int adsp_probe(struct platform_device *pdev)
>   	adsp_pds_detach(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>   free_rproc:
>   	rproc_free(rproc);
> +	device_init_wakeup(adsp->dev, false);

You would want to do ^^ before rproc free since adsp is a
private data of rproc struct.

>   
>   	return ret;
>   }
> @@ -573,6 +574,7 @@ static int adsp_remove(struct platform_device *pdev)
>   	qcom_remove_smd_subdev(adsp->rproc, &adsp->smd_subdev);
>   	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
>   	rproc_free(adsp->rproc);
> +	device_init_wakeup(adsp->dev, false);

ditto.

>   
>   	return 0;
>   }

- Sibi

Re: [PATCH 1/2] remoteproc: qcom_q6v5_pas: disable wakeup on probe fail or remove

[Mukesh Ojha]

Hi,

On 11/11/2022 5:24 PM, Luca Weiss wrote:
> Leaving wakeup enabled during probe fail (-EPROBE_DEFER) or remove makes
> the subsequent probe fail.
> 
> [    3.749454] remoteproc remoteproc0: releasing 3000000.remoteproc
> [    3.752949] qcom_q6v5_pas: probe of 3000000.remoteproc failed with error -17
> [    3.878935] remoteproc remoteproc0: releasing 4080000.remoteproc
> [    3.887602] qcom_q6v5_pas: probe of 4080000.remoteproc failed with error -17
> [    4.319552] remoteproc remoteproc0: releasing 8300000.remoteproc
> [    4.332716] qcom_q6v5_pas: probe of 8300000.remoteproc failed with error -17
> 
> Fix this by disabling wakeup in both cases so the driver can properly
> probe on the next try.
> 
> Fixes: a781e5aa5911 ("remoteproc: core: Prevent system suspend during remoteproc recovery")
> Fixes: dc86c129b4fb ("remoteproc: qcom: pas: Mark devices as wakeup capable")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

LGTM.
Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh
> ---
>   drivers/remoteproc/qcom_q6v5_pas.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 6afd0941e552..d830bf13c32c 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -557,6 +557,7 @@ static int adsp_probe(struct platform_device *pdev)
>   	adsp_pds_detach(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>   free_rproc:
>   	rproc_free(rproc);
> +	device_init_wakeup(adsp->dev, false);
>   
>   	return ret;
>   }
> @@ -573,6 +574,7 @@ static int adsp_remove(struct platform_device *pdev)
>   	qcom_remove_smd_subdev(adsp->rproc, &adsp->smd_subdev);
>   	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
>   	rproc_free(adsp->rproc);
> +	device_init_wakeup(adsp->dev, false);
>   
>   	return 0;
>   }

Re: [PATCH 2/2] remoteproc: qcom_q6v5_pas: detach power domains on remove

[Mukesh Ojha]

Hi,

On 11/11/2022 5:24 PM, Luca Weiss wrote:
> We need to detach from the power domains also on remove, not just on
> probe fail so a subsequent probe works as expected.
> 
> Otherwise the following error appears on re-probe:
> 
> [   29.452005] sysfs: cannot create duplicate filename '/devices/genpd:0:3000000.remoteproc'
> [   29.477121] CPU: 1 PID: 483 Comm: sh Tainted: G        W          6.1.0-rc4-00075-g71a113770bda #78
> [   29.510319] Hardware name: Fairphone 4 (DT)
> [   29.538335] Call trace:
> [   29.564470]  dump_backtrace.part.0+0xe0/0xf0
> [   29.592602]  show_stack+0x18/0x30
> [   29.619616]  dump_stack_lvl+0x64/0x80
> [   29.646834]  dump_stack+0x18/0x34
> [   29.673541]  sysfs_warn_dup+0x60/0x7c
> [   29.700592]  sysfs_create_dir_ns+0xec/0x110
> [   29.728057]  kobject_add_internal+0xb8/0x374
> [   29.755530]  kobject_add+0x9c/0x104
> [   29.782072]  device_add+0xbc/0x8a0
> [   29.808445]  device_register+0x20/0x30
> [   29.835175]  genpd_dev_pm_attach_by_id+0xa4/0x190
> [   29.862851]  genpd_dev_pm_attach_by_name+0x3c/0xb0
> [   29.890472]  dev_pm_domain_attach_by_name+0x20/0x30
> [   29.918212]  adsp_probe+0x278/0x580
> [   29.944384]  platform_probe+0x68/0xc0
> [   29.970603]  really_probe+0xbc/0x2dc
> [   29.996662]  __driver_probe_device+0x78/0xe0
> [   30.023491]  device_driver_attach+0x48/0xac
> [   30.050215]  bind_store+0xb8/0x114
> [   30.075957]  drv_attr_store+0x24/0x3c
> [   30.101874]  sysfs_kf_write+0x44/0x54
> [   30.127751]  kernfs_fop_write_iter+0x120/0x1f0
> [   30.154448]  vfs_write+0x1ac/0x380
> [   30.179937]  ksys_write+0x70/0x104
> [   30.205274]  __arm64_sys_write+0x1c/0x2c
> [   30.231060]  invoke_syscall+0x48/0x114
> [   30.256594]  el0_svc_common.constprop.0+0x44/0xec
> [   30.283183]  do_el0_svc+0x2c/0xd0
> [   30.308320]  el0_svc+0x2c/0x84
> [   30.333059]  el0t_64_sync_handler+0xf4/0x120
> [   30.359001]  el0t_64_sync+0x18c/0x190
> [   30.384385] kobject_add_internal failed for genpd:0:3000000.remoteproc with -EEXIST, don't try to register things with the same name in the same directory.
> [   30.406029] remoteproc remoteproc0: releasing 3000000.remoteproc
> [   30.416064] qcom_q6v5_pas: probe of 3000000.remoteproc failed with error -17
> 
> Fixes: 17ee2fb4e856 ("remoteproc: qcom: pas: Vote for active/proxy power domains")
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

LGTM.
Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh
> ---
>   drivers/remoteproc/qcom_q6v5_pas.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index d830bf13c32c..747155c85085 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -573,6 +573,7 @@ static int adsp_remove(struct platform_device *pdev)
>   	qcom_remove_sysmon_subdev(adsp->sysmon);
>   	qcom_remove_smd_subdev(adsp->rproc, &adsp->smd_subdev);
>   	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
> +	adsp_pds_detach(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>   	rproc_free(adsp->rproc);
>   	device_init_wakeup(adsp->dev, false);
>