Re: [PATCH] qmi: radio-settings: Do not unconditionally try to enable unsupported modes

[Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>]

Hi Denis,


On 11.12.24 г. 7:13 ч., Denis Kenzior wrote:

> 
> This looks like copy-paste of get_caps_cb.  Lets avoid that by invoking 
> QMI_DMS_GET_CAPS during probe().  See below.
> 

I was looking into doing it during probe, somehow missed 
OFONO_ATOM_DRIVER_FLAG_REGISTER_ON_PROBE flag. Now I see.

...

+    if (rsd->rat_mode_any || !get_rat_mode_any(rs, mode, cb, user_data))
> 
> So your intent here is to query the radio capabilities first if they 
> haven't been queried before?  If so, then the typical pattern is to do 
> this during probe(), before calling ofono_radio_settings_register().  
> See qmimodem/lte.c for an example.
> 

Exactly(the intent), but will do it like in lte.c

...

>>       available_rats = 0;
>> +
>>       for (i = 0; i < caps->radio_if_count; i++) {
>>           switch (caps->radio_if[i]) {
>>           case QMI_DMS_RADIO_IF_GSM:
>>               available_rats |= OFONO_RADIO_ACCESS_MODE_GSM;
>> +            rsd->rat_mode_any |= QMI_NAS_RAT_MODE_PREF_GSM;
>>               break;
>>           case QMI_DMS_RADIO_IF_UMTS:
>>               available_rats |= OFONO_RADIO_ACCESS_MODE_UMTS;
>> +            rsd->rat_mode_any |= QMI_NAS_RAT_MODE_PREF_UMTS;
>>               break;
>>           case QMI_DMS_RADIO_IF_LTE:
>>               available_rats |= OFONO_RADIO_ACCESS_MODE_LTE;
>> +            rsd->rat_mode_any |= QMI_NAS_RAT_MODE_PREF_LTE;
>>               break;
>>           }
>>       }
> 
> Wouldn't it be easier to simply do
> rsd->rat_mode_any = available_rats?
> 

No, because available_rats are of type OFONO_RADIO_ACCESS_TYPE_XXX, 
while rat_mode_any is of type QMI_NAS_RAT_MODE_PREF_XXX

Will send v2 with the above issues fixed.

Regards,
Ivo