Re: [PATCH net-next v4 08/13] net: wwan: t7xx: Add data path interface
flat view
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Date: 2022-02-03 14:24:06
Also in:
linux-wireless
On Thu, 13 Jan 2022, Ricardo Martinez wrote:
From: Haijun Liu <haijun.liu@mediatek.com> Data Path Modem AP Interface (DPMAIF) HIF layer provides methods for initialization, ISR, control and event handling of TX/RX flows. DPMAIF TX Exposes the `dmpaif_tx_send_skb` function which can be used by the network device to transmit packets. The uplink data management uses a Descriptor Ring Buffer (DRB). First DRB entry is a message type that will be followed by 1 or more normal DRB entries. Message type DRB will hold the skb information and each normal DRB entry holds a pointer to the skb payload. DPMAIF RX The downlink buffer management uses Buffer Address Table (BAT) and Packet Information Table (PIT) rings. The BAT ring holds the address of skb data buffer for the HW to use, while the PIT contains metadata about a whole network packet including a reference to the BAT entry holding the data buffer address. The driver reads the PIT and BAT entries written by the modem, when reaching a threshold, the driver will reload the PIT and BAT rings. Signed-off-by: Haijun Liu <haijun.liu@mediatek.com> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com> ---
+ unsigned short last_ch_id;
Values is never used.
quoted hunk ↗ jump to hunk
+ if (old_rl_idx > old_wr_idx && new_wr_idx >= old_rl_idx) { + dev_err(dpmaif_ctrl->dev, "RX BAT flow check fail\n"); + return -EINVAL; + } + + if (new_wr_idx >= bat_req->bat_size_cnt) { + new_wr_idx -= bat_req->bat_size_cnt; + if (new_wr_idx >= old_rl_idx) { + dev_err(dpmaif_ctrl->dev, "RX BAT flow check fail\n"); + return -EINVAL; + }
Make a label for the identical block and goto there.
quoted hunk ↗ jump to hunk
+static void t7xx_unmap_bat_skb(struct device *dev, struct dpmaif_bat_skb *bat_skb_base, + unsigned int index) +{ + struct dpmaif_bat_skb *bat_skb = bat_skb_base + index; + + if (bat_skb->skb) { + dma_unmap_single(dev, bat_skb->data_bus_addr, bat_skb->data_len, DMA_FROM_DEVICE); + kfree_skb(bat_skb->skb);
For consistency, dev_kfree_skb?
quoted hunk ↗ jump to hunk
+ * @initial: Indicates if the ring is being populated for the first time. + * + * Allocate skb and store the start address of the data buffer into the BAT ring. + * If this is not the initial call, notify the HW about the new entries. + * + * Return: + * * 0 - Success. + * * -ERROR - Error code from failure sub-initializations. + */ +int t7xx_dpmaif_rx_buf_alloc(struct dpmaif_ctrl *dpmaif_ctrl, + const struct dpmaif_bat_request *bat_req, + const unsigned char q_num, const unsigned int buf_cnt, + const bool initial)
vs its prototype: +int t7xx_dpmaif_rx_buf_alloc(struct dpmaif_ctrl *dpmaif_ctrl, + const struct dpmaif_bat_request *bat_req, const unsigned char q_num, + const unsigned int buf_cnt, const bool first_time);
quoted hunk ↗ jump to hunk
+int t7xx_dpmaif_rx_frag_alloc(struct dpmaif_ctrl *dpmaif_ctrl, struct dpmaif_bat_request *bat_req, + const unsigned int buf_cnt, const bool initial) +{ + struct dpmaif_bat_page *bat_skb = bat_req->bat_skb; + unsigned short cur_bat_idx = bat_req->bat_wr_idx; + unsigned int buf_space; + int ret, i;
...
+ ret = i < buf_cnt ? -ENOMEM : 0;
+ if (ret && initial) {
int ret = 0, i;
...
if (i < buf_cnt) {
ret = -ENOMEM;
if (initial) {
...
}
}
quoted hunk ↗ jump to hunk
+ if (!tx_drb_available || txq->tx_submit_skb_cnt >= txq->tx_list_max_len) { + cb = dpmaif_ctrl->callbacks; + cb->state_notify(dpmaif_ctrl->t7xx_dev, DMPAIF_TXQ_STATE_FULL, txqt); + return -EBUSY; + } + + skb->cb[TX_CB_QTYPE] = txqt; + skb->cb[TX_CB_DRB_CNT] = send_drb_cnt; + + spin_lock_irqsave(&txq->tx_skb_lock, flags); + list_add_tail(&skb->list, &txq->tx_skb_queue); + txq->tx_submit_skb_cnt++; + spin_unlock_irqrestore(&txq->tx_skb_lock, flags);
Perhaps the critical section needs to start earlier to enforce that tx_list_max_len check? (I'm yet to read half of this patch...) -- i.