On Thu, 13 Jan 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@xxxxxxxxx>
> Co-developed-by: Ricardo Martinez <ricardo.martinez@xxxxxxxxxxxxxxx>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@xxxxxxxxxxxxxxx>
> ---
> + unsigned short last_ch_id;
Values is never used.
> + 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.
> +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?
> + * @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);
> +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) {
...
}
}
> + 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.
