Re: [PATCH net-next v5 05/13] net: wwan: t7xx: Add control port

["Martinez, Ricardo" <ricardo.martinez@linux.intel.com>]

Hi Sergey,

On 3/6/2022 6:55 PM, Sergey Ryazanov wrote:
> On Thu, Feb 24, 2022 at 1:35 AM Ricardo Martinez
> <ricardo.martinez@linux.intel.com> wrote:
>> From: Haijun Liu <haijun.liu@mediatek.com>
>>
>> Control Port implements driver control messages such as modem-host
>> handshaking, controls port enumeration, and handles exception messages.
>>
>> The handshaking process between the driver and the modem happens during
>> the init sequence. The process involves the exchange of a list of
>> supported runtime features to make sure that modem and host are ready
>> to provide proper feature lists including port enumeration. Further
>> features can be enabled and controlled in this handshaking process.
>>
...
>> +static void t7xx_core_hk_handler(struct t7xx_modem *md, struct t7xx_fsm_ctl *ctl,
>> +                                enum t7xx_fsm_event_state event_id,
>> +                                enum t7xx_fsm_event_state err_detect)
>> +{
>> +       struct t7xx_sys_info *core_info = &md->core_md;
>> +       struct device *dev = &md->t7xx_dev->pdev->dev;
>> +       struct t7xx_fsm_event *event, *event_next;
>> +       unsigned long flags;
>> +       void *event_data;
>> +       int ret;
>> +
>> +       t7xx_prepare_host_rt_data_query(core_info);
>> +
>> +       while (!kthread_should_stop()) {
>> +               bool event_received = false;
>> +
>> +               spin_lock_irqsave(&ctl->event_lock, flags);
>> +               list_for_each_entry_safe(event, event_next, &ctl->event_queue, entry) {
>> +                       if (event->event_id == err_detect) {
>> +                               list_del(&event->entry);
>> +                               spin_unlock_irqrestore(&ctl->event_lock, flags);
>> +                               dev_err(dev, "Core handshake error event received\n");
>> +                               goto err_free_event;
>> +                       } else if (event->event_id == event_id) {
>> +                               list_del(&event->entry);
>> +                               event_received = true;
>> +                               break;
>> +                       }
>> +               }
>> +               spin_unlock_irqrestore(&ctl->event_lock, flags);
>> +
>> +               if (event_received)
>> +                       break;
>> +
>> +               wait_event_interruptible(ctl->event_wq, !list_empty(&ctl->event_queue) ||
>> +                                        kthread_should_stop());
>> +               if (kthread_should_stop())
>> +                       goto err_free_event;
>> +       }
>> +
>> +       if (ctl->exp_flg)
>> +               goto err_free_event;
>> +
>> +       event_data = (void *)event + sizeof(*event);
> In the V2, the event structure has a data field. But then it was
> dropped and now the attached data offset is manually calculated. Why
> did you do this, why event->data is not suitable here?

It was removed along with other zero length arrays, although it was 
declared as an empty array.

The next iteration will use C99 flexible arrays where required, instead 
of calculating the data offset manually.

...