Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

drivers: modem: gsm: gsm_ppp_stop() does not change gsm->state #51505

Closed
jeppenodgaard opened this issue Oct 21, 2022 · 3 comments · Fixed by #51792
Closed

drivers: modem: gsm: gsm_ppp_stop() does not change gsm->state #51505

jeppenodgaard opened this issue Oct 21, 2022 · 3 comments · Fixed by #51792
Assignees
@rerickson1
Labels
area: Modem bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug Regression Something, which was working, does not anymore

Comments

@jeppenodgaard
Copy link

jeppenodgaard commented Oct 21, 2022 •
edited by ycsin

Describe the bug
gsm_ppp_stop() does not set gsm->state to GSM_PPP_STOP.

Please also mention any information which could help others to understand
the problem you're facing:
I want to restart gsm_ppp by calling gsm_ppp_stop() then gsm_ppp_start(). However gsm_ppp_start() does not run because gsm->state is not GSM_PPP_STOP.

To Reproduce
Steps to reproduce the behavior:
When gsm_ppp is in gsm->state GSM_PPP_SETUP_DONE call gsm_ppp_stop() then gsm_ppp_start().

Expected behavior
gsm_ppp_start() runs normally.

Impact
It is not possible to start gsm_ppp again if e.g. NO CARRIER is received.

Logs and console output

[00:01:54.403,503] <inf> modem_gsm: GSM network interface down
[00:01:54.403,594] <err> modem_gsm: gsm_ppp is already started
[00:03:04.419,921] <wrn> modem_gsm: Failed locking modem cmds!
[00:03:04.419,921] <err> modem_gsm: gsm_ppp is already started
[00:04:14.436,279] <wrn> modem_gsm: Failed locking modem cmds!
[00:04:14.436,309] <err> modem_gsm: gsm_ppp is already started
[00:05:24.452,636] <wrn> modem_gsm: Failed locking modem cmds!
[00:05:24.452,636] <err> modem_gsm: gsm_ppp is already started

Environment (please complete the following information):

#51505 (comment)
@jeppenodgaard jeppenodgaard added the bug The issue is a bug, or the PR is fixing a bug label Oct 21, 2022
@laurenmurphyx64 laurenmurphyx64 added the priority: low Low impact/importance bug label Oct 25, 2022
@jeppenodgaard
Copy link
Author

To me it looks like this line gsm->state = GSM_PPP_STOP is missing. Do you have any feedback @ycsin ?

@ycsin ycsin added Regression Something, which was working, does not anymore priority: medium Medium impact/importance bug and removed priority: low Low impact/importance bug labels Oct 31, 2022
@ycsin
Copy link
Collaborator

ycsin commented Oct 31, 2022 •
edited

@jeppenodgaard yes you are right, somehow the reset is missing from my FSM implementation PR.
In https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/modem/gsm_ppp.c#L1212, the state should be reset to GSM_PPP_STOP, would it be possible for you to submit a patch for this?

@jeppenodgaard jeppenodgaard mentioned this issue Oct 31, 2022
Merged
@jeppenodgaard
Copy link
Author

@ycsin Yes :) Created a PR.

fabiobaltieri pushed a commit that referenced this issue Nov 1, 2022
@fabiobaltieri
drivers: modem: gsm: Fix set gsm->state in gsm_ppp_stop
625efd8 
The driver should set the `gsm->state` to GSM_PPP_STOP when
`gsm_ppp_stop()` is run. This commit fixes that.

Fixes #51505

Signed-off-by: Jeppe Odgaard <jeppe.odgaard@prevas.dk>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this issue Nov 4, 2022
drivers: modem: gsm: Fix set gsm->state in gsm_ppp_stop
f7f868e 
The driver should set the `gsm->state` to GSM_PPP_STOP when
`gsm_ppp_stop()` is run. This commit fixes that.

Fixes zephyrproject-rtos/zephyr#51505

(cherry picked from commit 625efd8)

Original-Signed-off-by: Jeppe Odgaard <jeppe.odgaard@prevas.dk>
GitOrigin-RevId: 625efd8
Change-Id: I7f3e8bc87d2996d0b349217aecbc6847f9fdf947
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/3996788
Reviewed-by: Tristan Honscheid <honscheid@google.com>
Commit-Queue: Tristan Honscheid <honscheid@google.com>
Tested-by: CopyBot Service Account <copybot.service@gmail.com>
Tested-by: Tristan Honscheid <honscheid@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rerickson1 rerickson1

Labels
area: Modem bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug Regression Something, which was working, does not anymore
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

drivers: modem: gsm: Fix set gsm->state in gsm_ppp_stop
5 participants
@ycsin @jeppenodgaard @rerickson1 @laurenmurphyx64 @nordicjm