remoteproc: qcom_q6v5: don't auto boot remote processor
Subsystems (2)
PATCH 1/1 remoteproc: qcom_q6v5: don't auto boot remote processor
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
--- drivers/remoteproc/qcom_q6v5_pil.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/remoteproc/qcom_q6v5_pil.c +2 -0
@@ -1133,6 +1133,8 @@ static int q6v5_probe(struct platform_device *pdev)return -ENOMEM ; } + rproc->auto_boot = false; + qproc = (struct q6v5 *)rproc->priv; qproc->dev = &pdev->dev; qproc->rproc = rproc;
Signature · 1 line
2.17.0
Quoted 6 lines · Ramon Fried
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
Signed-off-by Ramon Fried <[email protected]>
Thanks for your patch Ramon. While this nudges the behavior to make things work slightly better I think we need to describe the explicit dependency between the mss firmware and the existence of rmtfs.
As our remoteprocs are essentially always-on I would prefer that they start "automatically" and not through use of the sysfs interface.
But we're at the point where this is a real problem on 410, 820 and 845, so we have to come up with some way to tie these pieces together. If your patch suits that solution I will happily take it.
Regards,
Bjorn
Quoted 20 lines
--- drivers/remoteproc/qcom_q6v5_pil.c | 2 ++ 1 file changed, 2 insertions(+)drivers/remoteproc/qcom_q6v5_pil.c +2
-0@@ -1133,6 +1133,8 @@ static int q6v5_probe(struct platform_device *pdev)return -ENOMEM ; } + rproc->auto_boot = false; + qproc = (struct q6v5 *)rproc->priv; qproc->dev = &pdev->dev; qproc->rproc = rproc;Signature · 1 line
2.17.0
Quoted 13 lines · Bjorn Andersson
Quoted 6 lines · Ramon Fried
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
Signed-off-by Ramon Fried <[email protected]>Thanks for your patch Ramon. While this nudges the behavior to make things work slightly better I think we need to describe the explicit dependency between the mss firmware and the existence of rmtfs.
Quoted 6 lines
As our remoteprocs are essentially always-on I would prefer that they start "automatically" and not through use of the sysfs interface.
But we're at the point where this is a real problem on 410, 820 and 845, so we have to come up with some way to tie these pieces together. If your patch suits that solution I will happily take it.
Yes. it was tested on 410, and it did the trick.
Quoted 24 lines
Regards,
BjornQuoted 20 lines
--- drivers/remoteproc/qcom_q6v5_pil.c | 2 ++ 1 file changed, 2 insertions(+)drivers/remoteproc/qcom_q6v5_pil.c +2
-1@@ -1133,6 +1133,8 @@ static int q6v5_probe(struct platform_device *pdev)return -ENOMEM ; } + rproc->auto_boot = false; + qproc = (struct q6v5 *)rproc->priv; qproc->dev = &pdev->dev; qproc->rproc = rproc; --2.17.0
Sibi Sankar
Acked-byQuoted 16 lines · Bjorn Andersson
Quoted 6 lines · Ramon Fried
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
Signed-off-by Ramon Fried <[email protected]>Thanks for your patch Ramon. While this nudges the behavior to make things work slightly better I think we need to describe the explicit dependency between the mss firmware and the existence of rmtfs.
As our remoteprocs are essentially always-on I would prefer that they start "automatically" and not through use of the sysfs interface.
Quoted 6 lines
But we're at the point where this is a real problem on 410, 820 and 845, so we have to come up with some way to tie these pieces together. If your patch suits that solution I will happily take it.
Regards,
Bjorn
After experimenting with in kernel solutions for three revisions and observing problems on graceful shutdown usecase, switching to controlling the remoteproc mss through rmtfs seems to solve all the known issues.
https://patchwork.kernel.org/patch/10662395/
we should probably get this merged in, now that
we are planning to start/stop mss through
rmtfs.
Quoted 22 lines
Quoted 22 lines
--- drivers/remoteproc/qcom_q6v5_pil.c | 2 ++ 1 file changed, 2 insertions(+)drivers/remoteproc/qcom_q6v5_pil.c +2
-0@@ -1133,6 +1133,8 @@ static int q6v5_probe(struct platform_device *pdev)return -ENOMEM ; } + rproc->auto_boot = false; + qproc = (struct q6v5 *)rproc->priv; qproc->dev = &pdev->dev;
qproc->rproc= rproc;
--
2.17.0
Signature · 3 lines
-- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 2 lines · Sibi Sankar
Quoted 1 line · Bjorn Andersson
On Thu 24 May 12:21 PDT 2018, Ramon Fried wrote:
Whoa, bringing up a 7-month old patch? Nice.
Quoted 19 lines
Quoted 19 lines
Quoted 6 lines
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
Signed-off-by Ramon Fried <[email protected]>Thanks for your patch Ramon. While this nudges the behavior to make things work slightly better I think we need to describe the explicit dependency between the mss firmware and the existence of rmtfs.
As our remoteprocs are essentially always-on I would prefer that they start "automatically" and not through use of the sysfs interface.
But we're at the point where this is a real problem on 410, 820 and 845,
so we have to come up with some way to tie these pieces together. If your patch suits that solution I will happily take it.
Quoted 6 lines
Quoted 2 lines
Regards,
BjornAfter experimenting with in kernel solutions for three revisions and observing problems on graceful shutdown usecase,
What exactly were the problems again? e.g., what were the deficiencies with having the remoteproc device listen for the REMOTEFS_QMI_SVC_ID service again? Sorry, but I sort of dropped off on reviewing that stuff, and now I see this. I'd mildly prefer something that is actually automatic, but if I'm missing some aspects, I'd like to hear that. (And, I'd like to see them explained in the commit message, if this is ever to be merged.)
Quoted 3 lines
switching to controlling the
remoteproc mss through rmtfs seems to solve all the known issues.
How so? It explicitly does NOT help at all if RMTFS crashes. Because...who's going to stop the modem in that case? (It works if you automatically respawn a new RMTFS daemon, to toggle the modem. But that's kind of cheating, and you can do that anyway, even without this patch.) On the contrary, your patch *would* resolve that, since the modem would notice when the RMTFS server goes away, and it would stop itself.
Quoted 5 lines
https://patchwork.kernel.org/patch/10662395/
we should probably get this merged in, now that we are planning to start/stop mss through rmtfs.
Sorry, who's planning to stop mss through rmtfs? Did I miss something?
Brian
Quoted 32 lines · Brian Norris
Quoted 2 lines · Sibi Sankar
Quoted 1 line · Bjorn Andersson
On Thu 24 May 12:21 PDT 2018, Ramon Fried wrote:
Whoa, bringing up a 7-month old patch? Nice.
Quoted 19 lines
Quoted 19 lines
Quoted 6 lines
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
Signed-off-by Ramon Fried <[email protected]>Thanks for your patch Ramon. While this nudges the behavior to make things work slightly better I think we need to describe the explicit dependency between the mss firmware and the existence of rmtfs.
As our remoteprocs are essentially always-on I would prefer that they start "automatically" and not through use of the sysfs interface.
But we're at the point where this is a real problem on 410, 820 and 845,
so we have to come up with some way to tie these pieces together. If your patch suits that solution I will happily take it.Quoted 6 lines
Quoted 2 lines
Regards,
BjornAfter experimenting with in kernel solutions for three revisions and observing problems on graceful shutdown usecase,
Quoted 7 lines
What exactly were the problems again? e.g., what were the deficiencies with having the remoteproc device listen for the
REMOTEFS_QMI_SVC_IDservice again? Sorry, but I sort of dropped off on reviewing that stuff, and now I see this. I'd mildly prefer something that is actually automatic, but if I'm missing some aspects, I'd like to hear that. (And, I'd like to see them explained in the commit message, if this is ever to be merged.)
bringing down the modem after the RMTFS server goes down leaves the modem in limbo (It has a few pending rmtfs transactions that cannot go through) which results in sysmon graceful shutdown failing. And we have to do a modem force-stop to proceed which we want to avoid in graceful shutdown cases. This is overcome by starting rproc mss from rmtfs after REMOTEFS_QMI service is up and stopping rproc mss from rmtfs on SIGKILL/SIGINT and other program error signals before bringing down the RMTFS_QMI service i.e before exiting the rmtfs server loop.
Quoted 5 lines
Quoted 4 lines
switching to controlling the
remoteproc mss through rmtfs seems to solve all the known issues.
Quoted 7 lines
How so? It explicitly does NOT help at all if RMTFS crashes. Because...who's going to stop the modem in that case? (It works if you automatically respawn a new RMTFS daemon, to toggle the modem. But that's kind of cheating, and you can do that anyway, even without this patch.) On the contrary, your patch *would* resolve that, since the modem would notice when the RMTFS server goes away, and it would stop itself.
yeah we would want to mimic what the kernel patch did with the exception of stopping modem before bringing down the rmtfs server (not toggle rproc state but start on rmtfs service up and stop before rmtfs server exit). So in that case we would not want the modem to auto-boot.
Quoted 8 lines
Quoted 6 lines
https://patchwork.kernel.org/patch/10662395/
we should probably get this merged in, now that we are planning to start/stop mss through rmtfs.
Sorry, who's planning to stop mss through rmtfs? Did I miss something?
I have a working patch which I'll soon send upstream for review, after it clears the internal reviews/processes
Quoted 2 lines
Brian
Signature · 3 lines
-- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Hi Sibi,
Quoted 14 lines · Sibi Sankar
Quoted 5 lines · Brian Norris
On Thu, Jan 17, 2019 at 11:04 PM Sibi Sankar <[email protected]>
Quoted 3 lines
After experimenting with in kernel solutions for three revisions and observing problems on graceful shutdown usecase,
Quoted 7 lines
What exactly were the problems again? e.g., what were the deficiencies with having the remoteproc device listen for the
REMOTEFS_QMI_SVC_IDservice again? Sorry, but I sort of dropped off on reviewing that stuff, and now I see this. I'd mildly prefer something that is actually automatic, but if I'm missing some aspects, I'd like to hear that. (And, I'd like to see them explained in the commit message, if this is ever to be merged.)
Quoted 4 lines
bringing down the modem after the RMTFS server goes down leaves the modem in limbo (It has a few pending rmtfs transactions that cannot go through) which results in sysmon graceful shutdown failing.
Makes sense. Probably should be described in a re-send of this patch, if we're going with that.
Quoted 2 lines
And we have to do a modem force-stop to proceed which we want to avoid in graceful shutdown cases.
Shouldn't you do the "force-stop" in the kernel too? e.g., if rmtfs daemon dies without doing a properly-timed stop, then we should still force a stop in the kernel, no? Basically, why not do both mechanisms:
REMOTEFS_QMI_SVC-activated start/stop in the kernel, and manual stop (and start? this likely might still be redundant) in the daemon?
Quoted 6 lines
This is overcome by starting rproc mss from rmtfs after
REMOTEFS_QMIservice is up and stopping rproc mss from rmtfs on SIGKILL/SIGINT and other program error signals before bringing down theRMTFS_QMIservice i.e before exiting the rmtfs server loop.
That's still not really a sure-fire way of doing things. For one, you can't catch SIGKILL. Similarly, you can't really clean up from OOM, segfault, etc. So it would still be helpful to hook into the QMI service notifications in the kernel, I think.
Quoted 12 lines
Quoted 11 lines
Quoted 3 lines
switching to controlling the
remoteproc mss through rmtfs seems to solve all the known issues.How so? It explicitly does NOT help at all if RMTFS crashes. Because...who's going to stop the modem in that case? (It works if you automatically respawn a new RMTFS daemon, to toggle the modem. But that's kind of cheating, and you can do that anyway, even without this patch.) On the contrary, your patch *would* resolve that, since the modem would notice when the RMTFS server goes away, and it would stop itself.
Quoted 6 lines
yeah we would want to mimic what the kernel patch did with the exception of stopping modem before bringing down the rmtfs server (not toggle rproc state but start on rmtfs service up and stop before rmtfs server exit). So in that case we would not want the modem to auto-boot.
See above. You can't really mimic what the kernel patch was doing completely. You can try (which is probably a good idea), but you should still have some fallback in the kernel, I expect.
Quoted 11 lines
Quoted 7 lines
Quoted 5 lines
https://patchwork.kernel.org/patch/10662395/
we should probably get this merged in, now that we are planning to start/stop mss through rmtfs.
Sorry, who's planning to stop mss through rmtfs? Did I miss something?
I have a working patch which I'll soon send upstream for review, after it clears the internal reviews/processes
OK.
Brian
Hi Brian,
Thanks for the review
Quoted 25 lines · Brian Norris
Hi Sibi,
Quoted 14 lines · Sibi Sankar
Quoted 5 lines · Brian Norris
On Thu, Jan 17, 2019 at 11:04 PM Sibi Sankar <[email protected]>
Quoted 3 lines
After experimenting with in kernel solutions for three revisions and observing problems on graceful shutdown usecase,
Quoted 7 lines
What exactly were the problems again? e.g., what were the deficiencies with having the remoteproc device listen for the
REMOTEFS_QMI_SVC_IDservice again? Sorry, but I sort of dropped off on reviewing that stuff, and now I see this. I'd mildly prefer something that is actually automatic, but if I'm missing some aspects, I'd like to hear that. (And, I'd like to see them explained in the commit message, if this is ever to be merged.)Quoted 4 lines
bringing down the modem after the RMTFS server goes down leaves the modem in limbo (It has a few pending rmtfs transactions that cannot go through) which results in sysmon graceful shutdown failing.
Makes sense. Probably should be described in a re-send of this patch, if we're going with that.
Quoted 8 lines
Quoted 2 lines
And we have to do a modem force-stop to proceed which we want to avoid in graceful shutdown cases.
Shouldn't you do the "force-stop" in the kernel too? e.g., if rmtfs daemon dies without doing a properly-timed stop, then we should still force a stop in the kernel, no? Basically, why not do both mechanisms:
REMOTEFS_QMI_SVC-activated start/stop in the kernel, and manual stop (and start? this likely might still be redundant) in the daemon?
yeah we already have that implemented in the kernel i.e first we try graceful shutdown followed by force-stop which will just timeout if graceful shutdown was already completed. We would just call stop from rmtfs without worrying about the underlying details.
Quoted 8 lines
Quoted 7 lines
This is overcome by starting rproc mss from rmtfs after
REMOTEFS_QMIservice is up and stopping rproc mss from rmtfs on SIGKILL/SIGINT and other program error signals before bringing down theRMTFS_QMIservice i.e before exiting the rmtfs server loop.
Quoted 4 lines
That's still not really a sure-fire way of doing things. For one, you can't catch SIGKILL. Similarly, you can't really clean up from OOM, segfault, etc. So it would still be helpful to hook into the QMI service notifications in the kernel, I think.
yes we would want a fail safe in the kernel
as well. Sorry I meant SIGTERM instead of
SIGKILL earlier
Quoted 20 lines
Quoted 13 lines
Quoted 5 lines
Quoted 4 lines
switching to controlling the
remoteproc mss through rmtfs seems to solve all the known issues.Quoted 7 lines
How so? It explicitly does NOT help at all if RMTFS crashes. Because...who's going to stop the modem in that case? (It works if you automatically respawn a new RMTFS daemon, to toggle the modem. But that's kind of cheating, and you can do that anyway, even without this patch.) On the contrary, your patch *would* resolve that, since the modem would notice when the RMTFS server goes away, and it would stop itself.
Quoted 6 lines
yeah we would want to mimic what the kernel patch did with the exception of stopping modem before bringing down the rmtfs server (not toggle rproc state but start on rmtfs service up and stop before rmtfs server exit). So in that case we would not want the modem to auto-boot.
Quoted 3 lines
See above. You can't really mimic what the kernel patch was doing completely. You can try (which is probably a good idea), but you should still have some fallback in the kernel, I expect.
yeah
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
https://patchwork.kernel.org/patch/10662395/
we should probably get this merged in, now that we are planning to start/stop mss through rmtfs.
Quoted 5 lines
Quoted 1 line
Sorry, who's planning to stop mss through rmtfs? Did I miss something?
I have a working patch which I'll soon send upstream for review, after it clears the internal reviews/processes
OK.
Brian
Signature · 3 lines
-- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 6 lines · Ramon Fried
Sometimes that rmtfs userspace module is not brought up fast enough and the modem crashes. disabling automated boot in the driver and triggering the boot from user-space sovles the problem.
Signed-off-by Ramon Fried <[email protected]>
While I don't think we have fully settled on the mechanism yet, it's clear that we don't want this to auto boot using the existing mechanism.
So I picked this patch, with Sibi's ack.
Thanks,
Bjorn
Quoted 20 lines
--- drivers/remoteproc/qcom_q6v5_pil.c | 2 ++ 1 file changed, 2 insertions(+)drivers/remoteproc/qcom_q6v5_pil.c +2
-0@@ -1133,6 +1133,8 @@ static int q6v5_probe(struct platform_device *pdev)return -ENOMEM ; } + rproc->auto_boot = false; + qproc = (struct q6v5 *)rproc->priv; qproc->dev = &pdev->dev; qproc->rproc = rproc;Signature · 1 line
2.17.0