On Thu, Feb 24, 2022 at 1:35 AM Ricardo Martinez
[off-list ref] wrote:

From: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

Adds AT and MBIM ports to the port proxy infrastructure.
The initialization method is responsible for creating the corresponding
ports using the WWAN framework infrastructure. The implemented WWAN port
operations are start, stop, and TX.

[skipped]

+static int t7xx_port_ctrl_tx(struct wwan_port *port, struct sk_buff *skb)
+{
+       struct t7xx_port *port_private = wwan_port_get_drvdata(port);
+       size_t actual_len, alloc_size, txq_mtu = CLDMA_MTU;
+       struct t7xx_port_static *port_static;
+       unsigned int len, i, packets;
+       struct t7xx_fsm_ctl *ctl;
+       enum md_state md_state;
+
+       len = skb->len;
+       if (!len || !port_private->rx_length_th || !port_private->chan_enable)
+               return -EINVAL;
+
+       port_static = port_private->port_static;
+       ctl = port_private->t7xx_dev->md->fsm_ctl;
+       md_state = t7xx_fsm_get_md_state(ctl);
+       if (md_state == MD_STATE_WAITING_FOR_HS1 || md_state == MD_STATE_WAITING_FOR_HS2) {
+               dev_warn(port_private->dev, "Cannot write to %s port when md_state=%d\n",
+                        port_static->name, md_state);
+               return -ENODEV;
+       }
+
+       alloc_size = min_t(size_t, txq_mtu, len + CCCI_HEADROOM);
+       actual_len = alloc_size - CCCI_HEADROOM;
+       packets = DIV_ROUND_UP(len, txq_mtu - CCCI_HEADROOM);
+
+       for (i = 0; i < packets; i++) {
+               struct ccci_header *ccci_h;
+               struct sk_buff *skb_ccci;
+               int ret;
+
+               if (packets > 1 && packets == i + 1) {
+                       actual_len = len % (txq_mtu - CCCI_HEADROOM);
+                       alloc_size = actual_len + CCCI_HEADROOM;
+               }

Why do you track the packet number? Why not track the offset in the
passed data? E.g.:

for (off = 0; off < len; off += chunklen) {
    chunklen = min(len - off, CLDMA_MTU - sizeof(struct ccci_header);
    skb_ccci = alloc_skb(chunklen + sizeof(struct ccci_header), ...);
    skb_put_data(skb_ccci, skb->data + off, chunklen);
    /* Send skb_ccci */
}

+               skb_ccci = __dev_alloc_skb(alloc_size, GFP_KERNEL);
+               if (!skb_ccci)
+                       return -ENOMEM;
+
+               ccci_h = skb_put(skb_ccci, sizeof(*ccci_h));
+               t7xx_ccci_header_init(ccci_h, 0, actual_len + sizeof(*ccci_h),
+                                     port_static->tx_ch, 0);
+               skb_put_data(skb_ccci, skb->data + i * (txq_mtu - CCCI_HEADROOM), actual_len);
+               t7xx_port_proxy_set_tx_seq_num(port_private, ccci_h);
+
+               ret = t7xx_port_send_skb_to_md(port_private, skb_ccci);
+               if (ret) {
+                       dev_kfree_skb_any(skb_ccci);
+                       dev_err(port_private->dev, "Write error on %s port, %d\n",
+                               port_static->name, ret);
+                       return ret;
+               }
+
+               port_private->seq_nums[MTK_TX]++;

Sequence number tracking as well as CCCI header construction are
common operations, so why not move them to t7xx_port_send_skb_to_md()?

+       }
+
+       dev_kfree_skb(skb);
+       return 0;
+}

--
Sergey