* [PATCH] tee: optee: Populate child nodes in probe function @ 2022-11-09 16:07 Ludvig Pärsson 2022-11-10 10:53 ` Sumit Garg 2022-11-11 16:13 ` Sudeep Holla 0 siblings, 2 replies; 7+ messages in thread From: Ludvig Pärsson @ 2022-11-09 16:07 UTC (permalink / raw) To: Jens Wiklander Cc: kernel, Ludvig Pärsson, Sumit Garg, op-tee, linux-kernel Currently there is no dependency between the "linaro,scmi-optee" driver and the tee_core. If the scmi-optee driver gets probed before the tee_bus_type is initialized, then we will get an unwanted error print. This patch enables putting scmi-optee nodes as children to the optee node in devicetree, which indirectly creates the missing dependency. Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> --- drivers/tee/optee/smc_abi.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c index a1c1fa1a9c28..be6f02fd5a7f 100644 --- a/drivers/tee/optee/smc_abi.c +++ b/drivers/tee/optee/smc_abi.c @@ -1533,6 +1533,11 @@ static int optee_probe(struct platform_device *pdev) if (rc) goto err_disable_shm_cache; + /* Populate any dependent child node (if any) */ + rc = devm_of_platform_populate(&pdev->dev); + if (rc) + goto err_disable_shm_cache; + pr_info("initialized driver\n"); return 0; -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tee: optee: Populate child nodes in probe function 2022-11-09 16:07 [PATCH] tee: optee: Populate child nodes in probe function Ludvig Pärsson @ 2022-11-10 10:53 ` Sumit Garg 2022-11-10 12:09 ` Ludvig Pärsson 2022-11-11 16:13 ` Sudeep Holla 1 sibling, 1 reply; 7+ messages in thread From: Sumit Garg @ 2022-11-10 10:53 UTC (permalink / raw) To: Ludvig Pärsson; +Cc: Jens Wiklander, kernel, op-tee, linux-kernel On Wed, 9 Nov 2022 at 21:37, Ludvig Pärsson <ludvig.parsson@axis.com> wrote: > > Currently there is no dependency between the "linaro,scmi-optee" driver > and the tee_core. If the scmi-optee driver gets probed before the > tee_bus_type is initialized, then we will get an unwanted error print. > What error print do you observe? I suppose this case is already handled by scmi optee driver via -EPROBE_DEFER. -Sumit > This patch enables putting scmi-optee nodes as children to the optee > node in devicetree, which indirectly creates the missing dependency. > > Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> > --- > drivers/tee/optee/smc_abi.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > index a1c1fa1a9c28..be6f02fd5a7f 100644 > --- a/drivers/tee/optee/smc_abi.c > +++ b/drivers/tee/optee/smc_abi.c > @@ -1533,6 +1533,11 @@ static int optee_probe(struct platform_device *pdev) > if (rc) > goto err_disable_shm_cache; > > + /* Populate any dependent child node (if any) */ > + rc = devm_of_platform_populate(&pdev->dev); > + if (rc) > + goto err_disable_shm_cache; > + > pr_info("initialized driver\n"); > return 0; > > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tee: optee: Populate child nodes in probe function 2022-11-10 10:53 ` Sumit Garg @ 2022-11-10 12:09 ` Ludvig Pärsson 2022-11-11 9:57 ` Sumit Garg 0 siblings, 1 reply; 7+ messages in thread From: Ludvig Pärsson @ 2022-11-10 12:09 UTC (permalink / raw) To: sumit.garg, Ludvig Pärsson Cc: jens.wiklander, linux-kernel, kernel, op-tee On Thu, 2022-11-10 at 16:23 +0530, Sumit Garg wrote: > On Wed, 9 Nov 2022 at 21:37, Ludvig Pärsson <ludvig.parsson@axis.com> > wrote: > > > > Currently there is no dependency between the "linaro,scmi-optee" > > driver > > and the tee_core. If the scmi-optee driver gets probed before the > > tee_bus_type is initialized, then we will get an unwanted error > > print. > > > > What error print do you observe? I suppose this case is already > handled by scmi optee driver via -EPROBE_DEFER. > > -Sumit > Hi Sumit, The error print is in driver_register(). This is kind of what happens: scmi_driver_init() scmi_probe() scmi_optee_link_supplier() scmi_optee_init() driver_register() <--- pr_err() if tee_bus_type is not initialized tee_init() <--- tee_bus_type gets initialized here The scmi_optee_link_supplier() will always return -EPROBE_DEFER the first time because scmi_optee_private is initialized in scmi_optee_service_probe, which is only called after the driver is registered in scmi_optee_init. Right now the driver_register fails because tee_bus_type is not initialized which is printing the unwanted error print. Another side effect of this is that scmi_optee_link_supplier() will return -EPROBE_DEFER a second time, and scmi_probe will be successful the third time instead of the second time. BR, Ludvig > > This patch enables putting scmi-optee nodes as children to the > > optee > > node in devicetree, which indirectly creates the missing > > dependency. > > > > Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> > > --- > > drivers/tee/optee/smc_abi.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/tee/optee/smc_abi.c > > b/drivers/tee/optee/smc_abi.c > > index a1c1fa1a9c28..be6f02fd5a7f 100644 > > --- a/drivers/tee/optee/smc_abi.c > > +++ b/drivers/tee/optee/smc_abi.c > > @@ -1533,6 +1533,11 @@ static int optee_probe(struct > > platform_device *pdev) > > if (rc) > > goto err_disable_shm_cache; > > > > + /* Populate any dependent child node (if any) */ > > + rc = devm_of_platform_populate(&pdev->dev); > > + if (rc) > > + goto err_disable_shm_cache; > > + > > pr_info("initialized driver\n"); > > return 0; > > > > -- > > 2.30.2 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tee: optee: Populate child nodes in probe function 2022-11-10 12:09 ` Ludvig Pärsson @ 2022-11-11 9:57 ` Sumit Garg 2022-11-11 13:16 ` Ludvig Pärsson 0 siblings, 1 reply; 7+ messages in thread From: Sumit Garg @ 2022-11-11 9:57 UTC (permalink / raw) To: Ludvig Pärsson; +Cc: jens.wiklander, linux-kernel, kernel, op-tee On Thu, 10 Nov 2022 at 17:39, Ludvig Pärsson <Ludvig.Parsson@axis.com> wrote: > > On Thu, 2022-11-10 at 16:23 +0530, Sumit Garg wrote: > > On Wed, 9 Nov 2022 at 21:37, Ludvig Pärsson <ludvig.parsson@axis.com> > > wrote: > > > > > > Currently there is no dependency between the "linaro,scmi-optee" > > > driver > > > and the tee_core. If the scmi-optee driver gets probed before the > > > tee_bus_type is initialized, then we will get an unwanted error > > > print. > > > > > > > What error print do you observe? I suppose this case is already > > handled by scmi optee driver via -EPROBE_DEFER. > > > > -Sumit > > > Hi Sumit, > > The error print is in driver_register(). > > This is kind of what happens: > scmi_driver_init() > scmi_probe() > scmi_optee_link_supplier() > scmi_optee_init() > driver_register() <--- pr_err() if tee_bus_type is not > initialized > tee_init() <--- tee_bus_type gets initialized here > > The scmi_optee_link_supplier() will always return -EPROBE_DEFER the > first time because scmi_optee_private is initialized in > scmi_optee_service_probe, which is only called after the driver is > registered in scmi_optee_init. Right now the driver_register fails > because tee_bus_type is not initialized which is printing the unwanted > error print. Another side effect of this is that > scmi_optee_link_supplier() will return -EPROBE_DEFER a second time, and > scmi_probe will be successful the third time instead of the second > time. Thanks for the report. It rather looks like an inter subsystem dependency problem. Check if this [1] patch resolves your problem? [1] https://lkml.org/lkml/2022/11/11/329 -Sumit > > BR, > Ludvig > > > > This patch enables putting scmi-optee nodes as children to the > > > optee > > > node in devicetree, which indirectly creates the missing > > > dependency. > > > > > > Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> > > > --- > > > drivers/tee/optee/smc_abi.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/tee/optee/smc_abi.c > > > b/drivers/tee/optee/smc_abi.c > > > index a1c1fa1a9c28..be6f02fd5a7f 100644 > > > --- a/drivers/tee/optee/smc_abi.c > > > +++ b/drivers/tee/optee/smc_abi.c > > > @@ -1533,6 +1533,11 @@ static int optee_probe(struct > > > platform_device *pdev) > > > if (rc) > > > goto err_disable_shm_cache; > > > > > > + /* Populate any dependent child node (if any) */ > > > + rc = devm_of_platform_populate(&pdev->dev); > > > + if (rc) > > > + goto err_disable_shm_cache; > > > + > > > pr_info("initialized driver\n"); > > > return 0; > > > > > > -- > > > 2.30.2 > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tee: optee: Populate child nodes in probe function 2022-11-11 9:57 ` Sumit Garg @ 2022-11-11 13:16 ` Ludvig Pärsson 0 siblings, 0 replies; 7+ messages in thread From: Ludvig Pärsson @ 2022-11-11 13:16 UTC (permalink / raw) To: sumit.garg, Ludvig Pärsson Cc: jens.wiklander, linux-kernel, kernel, op-tee On Fri, 2022-11-11 at 15:27 +0530, Sumit Garg wrote: > On Thu, 10 Nov 2022 at 17:39, Ludvig Pärsson > <Ludvig.Parsson@axis.com> wrote: > > > > On Thu, 2022-11-10 at 16:23 +0530, Sumit Garg wrote: > > > On Wed, 9 Nov 2022 at 21:37, Ludvig Pärsson < > > > ludvig.parsson@axis.com> > > > wrote: > > > > > > > > Currently there is no dependency between the "linaro,scmi- > > > > optee" > > > > driver > > > > and the tee_core. If the scmi-optee driver gets probed before > > > > the > > > > tee_bus_type is initialized, then we will get an unwanted error > > > > print. > > > > > > > > > > What error print do you observe? I suppose this case is already > > > handled by scmi optee driver via -EPROBE_DEFER. > > > > > > -Sumit > > > > > Hi Sumit, > > > > The error print is in driver_register(). > > > > This is kind of what happens: > > scmi_driver_init() > > scmi_probe() > > scmi_optee_link_supplier() > > scmi_optee_init() > > driver_register() <--- pr_err() if tee_bus_type is not > > initialized > > tee_init() <--- tee_bus_type gets initialized here > > > > The scmi_optee_link_supplier() will always return -EPROBE_DEFER the > > first time because scmi_optee_private is initialized in > > scmi_optee_service_probe, which is only called after the driver is > > registered in scmi_optee_init. Right now the driver_register fails > > because tee_bus_type is not initialized which is printing the > > unwanted > > error print. Another side effect of this is that > > scmi_optee_link_supplier() will return -EPROBE_DEFER a second time, > > and > > scmi_probe will be successful the third time instead of the second > > time. > > Thanks for the report. It rather looks like an inter subsystem > dependency problem. Check if this [1] patch resolves your problem? > > [1] https://lkml.org/lkml/2022/11/11/329 > > -Sumit > Yes, this solves the problem, but I don't think the maintainer for the SCMI subsystem likes the solution. Earlier this week I discussed this problem with Sudeep, Cristian and Etienne that are CCed in the submission you linked. Changing the initcall level was one of my suggestions which got instantly rejected. We agreed that the solution in the patch i submitted was probably the best one, but maybe they will change their minds. BR, Ludvig > > > > BR, > > Ludvig > > > > > > This patch enables putting scmi-optee nodes as children to the > > > > optee > > > > node in devicetree, which indirectly creates the missing > > > > dependency. > > > > > > > > Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> > > > > --- > > > > drivers/tee/optee/smc_abi.c | 5 +++++ > > > > 1 file changed, 5 insertions(+) > > > > > > > > diff --git a/drivers/tee/optee/smc_abi.c > > > > b/drivers/tee/optee/smc_abi.c > > > > index a1c1fa1a9c28..be6f02fd5a7f 100644 > > > > --- a/drivers/tee/optee/smc_abi.c > > > > +++ b/drivers/tee/optee/smc_abi.c > > > > @@ -1533,6 +1533,11 @@ static int optee_probe(struct > > > > platform_device *pdev) > > > > if (rc) > > > > goto err_disable_shm_cache; > > > > > > > > + /* Populate any dependent child node (if any) */ > > > > + rc = devm_of_platform_populate(&pdev->dev); > > > > + if (rc) > > > > + goto err_disable_shm_cache; > > > > + > > > > pr_info("initialized driver\n"); > > > > return 0; > > > > > > > > -- > > > > 2.30.2 > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tee: optee: Populate child nodes in probe function 2022-11-09 16:07 [PATCH] tee: optee: Populate child nodes in probe function Ludvig Pärsson 2022-11-10 10:53 ` Sumit Garg @ 2022-11-11 16:13 ` Sudeep Holla 2022-11-14 6:38 ` Sumit Garg 1 sibling, 1 reply; 7+ messages in thread From: Sudeep Holla @ 2022-11-11 16:13 UTC (permalink / raw) To: Ludvig Pärsson Cc: Jens Wiklander, kernel, Sumit Garg, Sudeep Holla, op-tee, linux-kernel On Wed, Nov 09, 2022 at 05:07:08PM +0100, Ludvig Pärsson wrote: > Currently there is no dependency between the "linaro,scmi-optee" driver > and the tee_core. If the scmi-optee driver gets probed before the > tee_bus_type is initialized, then we will get an unwanted error print. > > This patch enables putting scmi-optee nodes as children to the optee > node in devicetree, which indirectly creates the missing dependency. > > Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> > --- > drivers/tee/optee/smc_abi.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > index a1c1fa1a9c28..be6f02fd5a7f 100644 > --- a/drivers/tee/optee/smc_abi.c > +++ b/drivers/tee/optee/smc_abi.c > @@ -1533,6 +1533,11 @@ static int optee_probe(struct platform_device *pdev) > if (rc) > goto err_disable_shm_cache; > > + /* Populate any dependent child node (if any) */ > + rc = devm_of_platform_populate(&pdev->dev); > + if (rc) > + goto err_disable_shm_cache; > + While I agree with idea of populating dependent child nodes from here, I am not sure if it is OK to give error if that fails or to just continue as there may be other users(like the user-space) for the OPTEE in general. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tee: optee: Populate child nodes in probe function 2022-11-11 16:13 ` Sudeep Holla @ 2022-11-14 6:38 ` Sumit Garg 0 siblings, 0 replies; 7+ messages in thread From: Sumit Garg @ 2022-11-14 6:38 UTC (permalink / raw) To: Sudeep Holla Cc: Ludvig Pärsson, Jens Wiklander, kernel, op-tee, linux-kernel On Fri, 11 Nov 2022 at 21:43, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Wed, Nov 09, 2022 at 05:07:08PM +0100, Ludvig Pärsson wrote: > > Currently there is no dependency between the "linaro,scmi-optee" driver > > and the tee_core. If the scmi-optee driver gets probed before the > > tee_bus_type is initialized, then we will get an unwanted error print. > > > > This patch enables putting scmi-optee nodes as children to the optee > > node in devicetree, which indirectly creates the missing dependency. > > > > Signed-off-by: Ludvig Pärsson <ludvig.parsson@axis.com> > > --- > > drivers/tee/optee/smc_abi.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > > index a1c1fa1a9c28..be6f02fd5a7f 100644 > > --- a/drivers/tee/optee/smc_abi.c > > +++ b/drivers/tee/optee/smc_abi.c > > @@ -1533,6 +1533,11 @@ static int optee_probe(struct platform_device *pdev) > > if (rc) > > goto err_disable_shm_cache; > > > > + /* Populate any dependent child node (if any) */ > > + rc = devm_of_platform_populate(&pdev->dev); > > + if (rc) > > + goto err_disable_shm_cache; > > + > > While I agree with idea of populating dependent child nodes from here, > I am not sure if it is OK to give error if that fails or to just continue > as there may be other users(like the user-space) for the OPTEE in general. > This uncertainty is simply because the inter subsystem dependency issue can't be resolved by this. See my comment regarding FF-A ABI on the other thread [1] [1] https://lkml.org/lkml/2022/11/14/29 -Sumit > -- > Regards, > Sudeep ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-14 6:38 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-09 16:07 [PATCH] tee: optee: Populate child nodes in probe function Ludvig Pärsson 2022-11-10 10:53 ` Sumit Garg 2022-11-10 12:09 ` Ludvig Pärsson 2022-11-11 9:57 ` Sumit Garg 2022-11-11 13:16 ` Ludvig Pärsson 2022-11-11 16:13 ` Sudeep Holla 2022-11-14 6:38 ` Sumit Garg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.