Bug 96855 - PDC service implementation required to run HP lt4120 modem under linux
Summary: PDC service implementation required to run HP lt4120 modem under linux
Status: RESOLVED FIXED
Alias: None
Product: libqmi
Classification: Unclassified
Component: libqmi (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Dan Williams
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 96854
Blocks:
  Show dependency treegraph
 
Reported: 2016-07-07 22:58 UTC by Aliaksandr Barouski
Modified: 2016-10-26 21:54 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
PDC service implementation (83.39 KB, patch)
2016-07-07 22:58 UTC, Aliaksandr Barouski
Details | Splinter Review
Fix situation if no PDC config loaded (3.08 KB, patch)
2016-10-12 20:41 UTC, Aliaksandr Barouski
Details | Splinter Review
Fix situation if no PDC config loaded (3.02 KB, patch)
2016-10-12 23:47 UTC, Aliaksandr Barouski
Details | Splinter Review
Minor fixes (1.53 KB, patch)
2016-10-25 21:16 UTC, Aliaksandr Barouski
Details | Splinter Review
Minor fixes (3.03 KB, patch)
2016-10-25 21:17 UTC, Aliaksandr Barouski
Details | Splinter Review

Description Aliaksandr Barouski 2016-07-07 22:58:21 UTC
Created attachment 124953 [details] [review]
PDC service implementation

The patch in attachment implements PDC (Platform device configuration) service and implements few calls from it to configure hp4120 modem.

Example of usage: https://github.com/borovsky/x5-snapdragon-linux/blob/master/copy-firmware.sh
Comment 1 Aleksander Morgado 2016-09-07 14:06:22 UTC
Hey Aliaksandr,

I've pushed your patch, plus some additional follow up commits fixing some issues afterwards, to the "pdc" branch in the upstream git repository. Could you validate that the additional fixes don't break the logic you're using?
Comment 2 Aliaksandr Barouski 2016-10-12 20:41:40 UTC
Created attachment 127257 [details] [review]
Fix situation if no PDC config loaded

The changed version works good, and I have few small additions to the code.

1. I found situation where original code haven't worked as expected:
If modem have no uploaded configs (e.g. just after boot), it doesn't send indication with config list, so code hung up.

2. I've returned printable_id function. The return format of the function is the same to expected format for pdc-activate-config/pdc-deactivate-config/pdc-delete-config, so user can just copy/paste the config id.

Attached patch fixes both things.
Comment 3 Aliaksandr Barouski 2016-10-12 23:47:06 UTC
Created attachment 127262 [details] [review]
Fix situation if no PDC config loaded

Removed debug from patch
Comment 4 Aleksander Morgado 2016-10-24 11:03:14 UTC
Regarding your last patch:

The timeout logic looked mostly good; I've pushed to the "pdc" branch a custom version of the logic, where the timeout is removed once we get the indication (so that the timeout doesn't break the logic if we're listing the configurations one by one), and also using g_timeout_add_seconds() instead of g_timeout_add().

I don't agree about getting back printable_id(). I think we should use qmicli_get_raw_data_printable() because we already have it. Note that I've pushed a new commit to fix how the ID was being printed, please retry with this latest fix installed.

Your patch was also doing this logic change; which I haven't included in the branch because I don't think it makes sense. The number of configs loaded should be equal to the number of configs available, the extra check for ctx->configs_loaded doesn't make much sense I think, or does it? Please elaborate on why that was needed.
@@ -276,7 +298,7 @@ static void
 check_list_config_completed (void)
 {
     if (ctx->configs_loaded == ctx->config_list->len &&
-        ctx->ids_loaded) {
+        (ctx->configs_loaded == 0 || ctx->ids_loaded)) {
         print_configs (ctx->config_list);
         operation_shutdown (TRUE);
     }
Comment 5 Aliaksandr Barouski 2016-10-25 21:16:23 UTC
Created attachment 127544 [details] [review]
Minor fixes

I've changed logic a bit to support situation when there is no config activated.
In  this case indication generated with error NOT_PROVISIONED. I've added handling for the case.

Also I've reduced verbosity for debug messages and made small fix in config printing.
Comment 6 Aliaksandr Barouski 2016-10-25 21:17:19 UTC
Created attachment 127545 [details] [review]
Minor fixes
Comment 7 Aleksander Morgado 2016-10-26 13:05:14 UTC
Included your latest changes and merged to git master already, thanks!
Comment 8 Sylvain Pasche 2016-10-26 21:54:04 UTC
I recently updated my Gnome 3.22 jhbuild checkout and ModemManager fails to build:

In file included from /home/spasche/jhbuild_322/checkout/ModemManager/src/mm-modem-helpers-qmi.h:22:0,
                 from /home/spasche/jhbuild_322/checkout/ModemManager/src/mm-modem-helpers-qmi.c:16:
/home/spasche/jhbuild_322/install/include/libqmi-glib/libqmi-glib.h:58:27: fatal error: qmi-enums-pdc.h: No such file or directory
 #include "qmi-enums-pdc.h"
                         

It seems that this PDC change is not installing all headers?


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.