Discussion:
[PATCH] atmodem/sim: added vendor Gemalto
Denis Kenzior
2018-09-24 21:00:36 UTC
Permalink
Hi Giacinto,
also renamed the cinterion_* functions in gemalto_*.
Eventually, the vendor cinterion will be removed.
---
drivers/atmodem/sim.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
This is all great and all, but where are my patches for actually fixing
the use of VENDOR_CINTERION for plugins/gemalto.c?

Regards,
-Denis
Denis Kenzior
2018-09-25 03:48:07 UTC
Permalink
Hi Giacinto,
In the meanwhile, will you take this change (and related ones)?
No :)

Each change set needs to be self contained. I see no reason why you
can't make VENDOR_CINTERION changes now in the relevant places. Even if
you eventually plan to remove plugins/cinterion.c completely in the future.

I can only read code, not people's minds.

Regards,
-Denis
Denis Kenzior
2018-09-25 04:04:33 UTC
Permalink
Hi Giacinto,
But the VENDOR_CINTERION remains in the code, there are no changes for
the plugin.
That's exactly the point. You have taken out all VENDOR_CINTERION
logic, yet plugins/gemalto.c still sets VENDOR_CINTERION for the sim
atom. So now you broke that driver. Your change set is not
self-contained. If I apply this patch and you get hit by a bus
tomorrow, the plugin remains broken for everyone.

Regards,
-Denis
Denis Kenzior
2018-09-25 04:16:44 UTC
Permalink
Hi Giacinto,
               if (g_at_chat_send(sd->chat, "AT^SPIC", gemalto_spic_prefix,
                                       gemalto_spic_cb, cbd, g_free) > 0)
both are there, and the functions are renamed, but still there.
Or shall I duplicate the functions for the time being?
Ok, I stand corrected, I assumed you removed OFONO_VENDOR_CINTERION from
the sms driver.

But still, can we strive for self-contained change sets? Just take out
VENDOR_CINTERION use inside the sim driver and change over to
VENDOR_GEMALTO for any modem drivers that need it. That makes things
much easier to follow compared to doing only the sim driver changes now
and then flipping from one VENDOR to another at some later point.

Regards,
-Denis
Denis Kenzior
2018-09-25 04:27:10 UTC
Permalink
Hi Giacinto,
I will have to submit together then a file from atmodem and a change in > plugin/cinterion.c, to be self-contained.> Another exception to the
rule, I suppose, but I will do so.
No, there's no exception here. Change set = set of patches. So you
need to send me a series of 2 patches. First patch changes
drivers/atmodem/sim.c. Second patch changes plugins/cinterion.c or
plugins/gemalto.c or both.

Regards,
-Denis

Loading...