On Tue, Apr 26, 2022 at 10:30 AM Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote: > On Tue, 26 Apr 2022, Sergey Ryazanov wrote: >> On Fri, Apr 8, 2022 at 1:37 AM Ricardo Martinez >> <ricardo.martinez@xxxxxxxxxxxxxxx> wrote: >>> 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> >>> >>> From a WWAN framework perspective: >>> Reviewed-by: Loic Poulain <loic.poulain@xxxxxxxxxx> >> >> Reviewed-by: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx> >> >> and a small question below. >> >>> diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c >>> ... >>> +static bool t7xx_alloc_and_map_skb_info(const struct dpmaif_ctrl *dpmaif_ctrl, >>> + const unsigned int size, struct dpmaif_bat_skb *cur_skb) >>> +{ >>> + dma_addr_t data_bus_addr; >>> + struct sk_buff *skb; >>> + size_t data_len; >>> + >>> + skb = __dev_alloc_skb(size, GFP_KERNEL); >>> + if (!skb) >>> + return false; >>> + >>> + data_len = skb_end_pointer(skb) - skb->data; >> >> Earlier you use a nice t7xx_skb_data_area_size() function here, but >> now you opencode it. Is it a consequence of t7xx_common.h removing? >> >> I would even encourage you to make this function common and place it >> into include/linux/skbuff.h with a dedicated patch and call it >> something like skb_data_size(). Surprisingly, there is no such helper >> function in the kernel, and several other drivers will benefit from >> it: > > I agree other than the name. IMHO, skb_data_size sounds too much "data > size" which it exactly isn't but just how large the memory area is (we > already have "datalen" anyway and on language level, those two don't sound > different at all). The memory area allocated may or may not have actual > data in it, I suggested adding "area" into it. I agree, using the "area" word in the helper name gives more clue about its purpose, thanks. -- Sergey