Re: vendor models and options

[Giacinto Cifelli <gciofono@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4378 bytes --]

hi Denis,

this is the structure I would like to propose:

=========== in drivers/atmodem/vendor.h:

enum ofono_vendor {
OFONO_VENDOR_GENERIC = 0,
OFONO_VENDOR_CALYPSO,
OFONO_VENDOR_IFX,
OFONO_VENDOR_STE,
OFONO_VENDOR_MBM,
OFONO_VENDOR_GOBI,
OFONO_VENDOR_QUALCOMM,
OFONO_VENDOR_HSO,
OFONO_VENDOR_ZTE,
OFONO_VENDOR_HUAWEI,
OFONO_VENDOR_SIERRA,
OFONO_VENDOR_NOVATEL,
OFONO_VENDOR_WAVECOM,
OFONO_VENDOR_NOKIA,
OFONO_VENDOR_PHONESIM,
OFONO_VENDOR_TELIT,
OFONO_VENDOR_SPEEDUP,
OFONO_VENDOR_SAMSUNG,
OFONO_VENDOR_SIMCOM,
OFONO_VENDOR_ICERA,
OFONO_VENDOR_ALCATEL,
OFONO_VENDOR_QUECTEL,
OFONO_VENDOR_UBLOX,
OFONO_VENDOR_GEMALTO,
OFONO_VENDOR_XMM,
};


enum gemalto_auth_type {
OV_GEMALTO_AUTH_CGAUTH = 0,
OV_GEMALTO_AUTH_SGAUTH_PWD_USER = 1,
OV_GEMALTO_AUTH_SGAUTH_USER_PWD = 2,
};

struct ov_gemalto {
enum gemalto_auth_type auth;
gboolean vts_with_quotes;
};


struct ov_quectel {
gboolean use_qtrpin;
};

struct ov_simcom {
gboolean use_atd99;
};

struct ov_ublox {
gboolean toby_l2;
};

struct ov_wavecom {
gboolean short_cpms;
};

struct ofono_vendor_parameters {
enum ofono_vendor vendor;
unsigned int model;
union {
struct ov_gemalto gemalto;
struct ov_quectel quelctel;
struct ov_simcom simcom;
struct ov_ublox ublox;
struct ov_wavecom wavecom;
};
};

extern struct ofono_vendor_parameters ov_generic;


=========== in drivers/atmodem/atmodem.c:

struct ofono_vendor_parameters ov_generic = {
OFONO_VENDOR_GENERIC, 0
};


==========

and then change the *_create() functions to use 'struct
ofono_vendor_parameters *' instead of 'unsigned int vendor', with default
&ov_generic.

In the example above, I have shown differences for the various Gemalto
authentication commands (3 options), that  cross with the options for VTS.
And then most of the settings are common for Gemalto models (currently
factored with the vendor=OFONO_VENDOR_CINTERION).
If  tomorrow I have to declare models for these simple settings, I have to
add at least 6 with the current system, and having a full driver just
because of these two AT commands seems needless.

The code would be simplified because in general in a switch there will be
only 1 vendor, with the current system there would be 6.

Is it ok if I update the code with this vendor system? I will have to
submit the entire tree, not by subdirectories, to have a consistent and
compiling commit.

Regards,
Giacinto








On Fri, Sep 7, 2018 at 5:09 PM Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Giacinto,
>
> On 09/07/2018 03:09 AM, Giacinto Cifelli wrote:
> > Dear Denis, all,
> >
> > while preparing the Gemalto driver, I see that there is a potential
> > shortcoming with the vendor structure in the atmodem. I can set
> > OFONO_VENDOR_GEMALTO_model, but this could be a very long list for all
> > options to maintain. I see the tendency also for other manufacturers.
> > Would it be ok if I convert the vendor integer in a structure with model
> > and flags?
> > This would make the current code more compact and clearer.
>
> Feel free to suggest something more concrete illustrating how your
> proposal would result in more compact code.  It seems to me that it
> would be quite disruptive, but I always try to have an open mind :)
>
> But of course there are other 'creative' ways we can utilize to handle
> this as well.
>
> >
> > The alternative of cloning the atmodem is less tempting, because then it
> > needs constant monitoring of the atmodem to porting the features in it,
> > or conversely missing features in this general driver. One example is
> > for the indicators +CGREG/+CEREG/+C5GREG that we are adding in atmodem
> > (gprs.c), instead of the unique +CGREG that doesn't work anymore for LTE
> > (in the 27.007). If I add it in Gemalto driver, it will be missing in
> > the atmodem.
>
> Aren't these commands now standardized?  Why do you need sub-model
> information to handle this?
>
> Either way, it is quite hard to give you any suggestions without seeing
> concretely what you're trying to do.  Perhaps you want to submit what
> you have (or some subset that illustrates the issues you're having) to
> the mailing list (even if it isn't completely ready, just tag it as
> 'RFC') and then I can give you more concrete feedback.
>
> Regards,
> -Denis
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 7909 bytes --]