On Thu, May 21, 2020 at 08:32:07AM -0500, Denis Kenzior wrote:
> Hi Lars,
> 
> On 5/26/20 5:16 AM, poeschel(a)lemonage.de wrote:
> > From: Lars Poeschel <poeschel@lemonage.de>
> > 
> > The Quectel EC21 does only work correctly, if the mux channel used for
> > aux is the first mux channel. It does only put it's URC messages in the
> > first mux channel, so this has to be the aux channel in our case.
> > ---
> >   plugins/quectel.c | 51 +++++++++++++++++++++++++++++++++++------------
> >   1 file changed, 38 insertions(+), 13 deletions(-)
> > 
> > diff --git a/plugins/quectel.c b/plugins/quectel.c
> > index 1d312c45..9ff75516 100644
> > --- a/plugins/quectel.c
> > +++ b/plugins/quectel.c
> > @@ -847,18 +847,38 @@ static void cmux_gatmux(struct ofono_modem *modem)
> >   	g_at_mux_start(data->mux);
> > -	data->modem = create_chat(modem, "Modem: ");
> > -	if (!data->modem) {
> > -		ofono_error("failed to create modem channel");
> > -		close_serial(modem);
> > -		return;
> > -	}
> > +	if (data->model == QUECTEL_EC21) {
> > +		data->aux = create_chat(modem, "Aux: ");
> > -	data->aux = create_chat(modem, "Aux: ");
> > -	if (!data->aux) {
> > -		ofono_error("failed to create aux channel");
> > -		close_serial(modem);
> > -		return;
> > +		if (!data->aux) {
> > +			ofono_error("failed to create aux channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> > +
> > +		data->modem = create_chat(modem, "Modem: ");
> > +
> > +		if (!data->modem) {
> > +			ofono_error("failed to create modem channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> > +	} else {
> > +		data->modem = create_chat(modem, "Modem: ");
> > +
> > +		if (!data->modem) {
> > +			ofono_error("failed to create modem channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> > +
> > +		data->aux = create_chat(modem, "Aux: ");
> > +
> > +		if (!data->aux) {
> > +			ofono_error("failed to create aux channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> >   	}
> 
> Can we be smarter about this and just store the per-model creation sequence
> as an array or something?  Then have a loop that calls create_chat from the
> array info?  The proposed copy-pasting approach is not maintainable long
> term.

I will post a follow-up with a proposed solution tomorrow.

> >   	setup_aux(modem);
> > @@ -951,8 +971,13 @@ static void cmux_ngsm(struct ofono_modem *modem)
> >   	 * the kernel does not yet support mapping the underlying serial device
> >   	 * to its virtual gsm ttys, so hard-code gsmtty1 gsmtty2 for now
> >   	 */
> > -	ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
> > -	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
> > +	if (data->model == QUECTEL_EC21) {
> > +		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
> > +		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
> > +	} else {
> > +		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
> > +		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
> > +	}
> 
> Doesn't this break the logic in mux_ready_cb, particularly the 'check if the
> last virtual gsm tty's are created'

Well, I guess, yes. I missed indeed missed that. The follow-up will
contain a fix for this as well.

> >   	/* wait for gsmtty devices to appear */
> >   	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
> > 
> 
> Regards,
> -Denis

Thank you for reviewing and applying the other patches! :-)

Lars