Re: [PATCH v2] sim7100: query until AT channel is ready

Hi Sean,

On 6/17/25 4:26 AM, Sean Nyekjaer wrote:
> It's seen that the AT channel contains unknown data upon power on.
> Query the modem until it answers with OK. Create a timer to re-issue
> the AT command at regular intervals until the modem answers OK.
> 
> plugins/sim7100.c:sim7100_enable()
> plugins/sim7100.c:open_device() devkey=AT
> src/modem.c:get_modem_property() modem 0x261f20 property AT
> plugins/sim7100.c:open_device() devkey=PPP
> src/modem.c:get_modem_property() modem 0x261f20 property PPP
> AT: > AT\r
> AT: < \r\r*A
> PPP: < \r\r*A
> AT: < AT\r
> plugins/sim7100.c:init_timeout_cb() 0x261f20
> AT: > AT\r
> AT: < AT\r
> AT: < \r\nOK\r\n
> AT: > ATE0Q0V1\r
> AT: < ATE0Q0V1\r\r\nOK\r\n
> AT: > AT+CGMM\r
> AT: < \r\nA7672E-FASE\r\n\r\nOK\r\n
> plugins/sim7100.c:cgmm_cb() modem model: A7672E-FASE
> plugins/sim7100.c:cgmm_cb() before CFUN
> AT: > AT+CFUN=4\r
> AT: < \r\n+CGEV: ME DETACH\r\n\r\n+CGEV: NW PDN DEACT 1\r\n
> PPP: < \r\n+CGEV: ME DETACH\r\n\r\n+CGEV: NW PDN DEACT 1\r\n
> AT: < \r\nOK\r\n
> plugins/sim7100.c:cfun_set_on_cb()
> src/modem.c:modem_change_state() old state: 0, new state: 1
> ---
> Changes since v1:
>   - removed double empty line
>   - Remove timeout in remove()
>   - Simplified open_device()
> 
>   plugins/sim7100.c | 64 ++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 55 insertions(+), 9 deletions(-)
> 

<snip>

Looks good.  Just one thing to fixup I think:

> +static void init_timeout_cb(struct l_timeout *timeout, void *user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct sim7100_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	if (data->init_count++ >= 30) {
> +		ofono_error("failed to init modem after 30 attempts");
> +		close_serial(modem);

When you hit this condition, you should delete / NULL the timeout and tell oFono 
core that the power up failed.  Something like:

l_timeout_remove();
data->init_timeout = NULL;
ofono_modem_set_powered(modem, FALSE);

Otherwise, when you try to power up the modem again, .enable() will be called 
again, and you will leak the timeout.

> +		return;
> +	}
> +
> +	g_at_chat_retry(data->at, data->init_cmd);
> +	l_timeout_modify_ms(timeout, 500);
> +}
> +
>   static int open_device(struct ofono_modem *modem, char *devkey, GAtChat **chat)
>   {
>   	DBG("devkey=%s", devkey);

Regards,
-Denis