Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d7b6b005-91eb-f2aa-62e5-1f94e5253bfd@quicinc.com>
Date:   Thu, 17 Nov 2022 16:10:40 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH 2/2] remoteproc: qcom_q6v5_pas: detach power domains on
 remove
Content-Language: en-US
To:     Luca Weiss <luca.weiss@fairphone.com>,
        <linux-arm-msm@vger.kernel.org>
CC:     <~postmarketos/upstreaming@lists.sr.ht>,
        <phone-devel@vger.kernel.org>, Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Sibi Sankar <sibis@codeaurora.org>,
        <linux-remoteproc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20221111115414.153961-1-luca.weiss@fairphone.com>
 <20221111115414.153961-2-luca.weiss@fairphone.com>
From:   Mukesh Ojha <quic_mojha@quicinc.com>
In-Reply-To: <20221111115414.153961-2-luca.weiss@fairphone.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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);
>