-
Notifications
You must be signed in to change notification settings - Fork 7.7k
drivers: modem: generalize Quectel BG9X support #93105
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.
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
base: main
Are you sure you want to change the base?
drivers: modem: generalize Quectel BG9X support #93105
Conversation
Hello @guildeol-stra, and thank you very much for your first pull request to the Zephyr project! |
Hi, @rerickson1 I think the LTS versions could benefit from this. Can this also be backported? |
Does this issue exist in 3.7? |
Yes. We use 3.7 as our base version and we first identified the issue there. |
Can you please writeup a issue quick so it can be linked to this PR? |
984c80a
to
a99767a
Compare
Done. Added the link and also amended my commit message because this did not cause a compilation issue, just disabled the config implicitly. Please let me know if there's anything else needed. |
Looks good. We'll see if its possible to get this bug fix in for 4.2. If not, it will come after. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was by design :) the bg9x compat is a relic used only by the quectel-bg9x.c driver, modem_cellular (and zephyr in general) requires the devicetree to have specific device compats in the devicetree, like quectel,bg95 (which is why it was added and is used by the modem subsystem)
To elaborate a bit, quectel,bg9x
is a malformed devicetree compatible, not a driver :) quectel-bg9x.c and modem_ceullar.c are drivers. These support specific hardware, which is what devicetree compatibles specify. If you have a bg95, or bg96 (something matching the "wildcard" bg9x), that compatible has to be added to zephyr, and the modem_cellular.c driver expanded to support it :)
And this is why multiple sets of eyes is best! This isn't really a bug then, right? This should technically be a new feature? |
I believe so, but, looking at the commit, without modifications to modem_cellular.c, this PR would still not provide support for the bg9x compat, it would build the driver, but no driver would be instantiated, so maybe there's more to it? |
efe651c
to
7ff0c68
Compare
e2aef45
to
b9fd4ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quectel-bg9x.c
driver cannot be removed without first deprecating it.
Please see: https://docs.zephyrproject.org/latest/develop/api/api_lifecycle.html
Replace specific reference for Quectel BG95 in Modem Cellular with BG9x, since the setup code should apply for other members of this family. Delete the 'quectel,bg95.yaml' DTS binding in favor of a modified 'quectel,bg9x.yaml' that should work for modem_cellular.c and quectel-bg9x.c. Signed-off-by: Guilherme Costa <[email protected]>
The modem_cellular implementation should suffice to handle the base usage of generic BG9x family devices, so deprecate the quectel-bg9x.c implementation. Signed-off-by: Guilherme Costa <[email protected]>
b9fd4ee
to
c601d43
Compare
|
Replace specific reference for Quectel BG95 in Modem Cellular with
BG9x, since the setup code should apply for other members of this
family.
Rename the 'quectel,bg95' DTS binding to 'quectel,bg9x' and add
optional 'mdm-reset-gpios' property.
To avoid confusion, delete the old 'quectel,bg9x' DTS binding and its
implementation.
Fixes #93115