> -----Original Message----- > From: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> > Sent: Wednesday, December 28, 2022 7:30 AM > To: linux-wireless@xxxxxxxxxxxxxxx > Cc: Yan-Hsuan Chuang <tony0620emma@xxxxxxxxx>; Kalle Valo <kvalo@xxxxxxxxxx>; Ulf Hansson > <ulf.hansson@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > linux-mmc@xxxxxxxxxxxxxxx; Chris Morgan <macroalpha82@xxxxxxxxx>; Nitin Gupta <nitin.gupta981@xxxxxxxxx>; > Neo Jou <neojou@xxxxxxxxx>; Ping-Ke Shih <pkshih@xxxxxxxxxxx>; Jernej Skrabec <jernej.skrabec@xxxxxxxxx>; > Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> > Subject: [RFC PATCH v1 12/19] rtw88: sdio: Add HCI implementation for SDIO based chipsets > > Add a sub-driver for SDIO based chipsets which implements the following > functionality: > - register accessors for 8, 16 and 32 bits for all states of the card > (including usage of 4x 8 bit access for one 32 bit buffer if the card > is not fully powered on yet - or if it's fully powered on then 1x 32 > bit access is used) > - checking whether there's space in the TX FIFO queue to transmit data > - transfers from the host to the device for actual network traffic, > reserved pages (for firmware download) and H2C (host-to-card) > transfers > - receiving data from the device > - deep power saving state > > The transmit path is optimized so DMA-capable SDIO host controllers can > directly use the buffers provided because the buffer's physical > addresses are 8 byte aligned. > > The receive path is prepared to support RX aggregation where the > chipset combines multiple MAC frames into one bigger buffer to reduce > SDIO transfer overhead. > > Co-developed-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx> > Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> > --- > drivers/net/wireless/realtek/rtw88/Kconfig | 3 + > drivers/net/wireless/realtek/rtw88/Makefile | 3 + > drivers/net/wireless/realtek/rtw88/debug.h | 1 + > drivers/net/wireless/realtek/rtw88/mac.h | 1 - > drivers/net/wireless/realtek/rtw88/reg.h | 10 + > drivers/net/wireless/realtek/rtw88/sdio.c | 1242 +++++++++++++++++++ > drivers/net/wireless/realtek/rtw88/sdio.h | 175 +++ > 7 files changed, 1434 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/wireless/realtek/rtw88/sdio.c > create mode 100644 drivers/net/wireless/realtek/rtw88/sdio.h > [...] > + > +static void rtw_sdio_writel(struct rtw_sdio *rtwsdio, u32 val, > + u32 addr, int *ret) > +{ > + u8 buf[4]; > + int i; > + > + if (!(addr & 3) && rtwsdio->is_powered_on) { > + sdio_writel(rtwsdio->sdio_func, val, addr, ret); > + return; > + } > + > + *(__le32 *)buf = cpu_to_le32(val); > + > + for (i = 0; i < 4; i++) { > + sdio_writeb(rtwsdio->sdio_func, buf[i], addr + i, ret); > + if (*ret) Do you need some messages to know something wrong? > + return; > + } > +} > + > +static u32 rtw_sdio_readl(struct rtw_sdio *rtwsdio, u32 addr, int *ret) > +{ > + u8 buf[4]; > + int i; > + > + if (!(addr & 3) && rtwsdio->is_powered_on) > + return sdio_readl(rtwsdio->sdio_func, addr, ret); > + > + for (i = 0; i < 4; i++) { > + buf[i] = sdio_readb(rtwsdio->sdio_func, addr + i, ret); > + if (*ret) > + return 0; > + } > + > + return le32_to_cpu(*(__le32 *)buf); > +} > + > +static u8 rtw_sdio_read_indirect8(struct rtw_dev *rtwdev, u32 addr, int *ret) > +{ > + struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv; > + u32 reg_cfg, reg_data; > + int retry; > + u8 tmp; > + > + reg_cfg = rtw_sdio_to_bus_offset(rtwdev, REG_SDIO_INDIRECT_REG_CFG); > + reg_data = rtw_sdio_to_bus_offset(rtwdev, REG_SDIO_INDIRECT_REG_DATA); > + > + rtw_sdio_writel(rtwsdio, BIT(19) | addr, reg_cfg, ret); > + if (*ret) > + return 0; > + > + for (retry = 0; retry < RTW_SDIO_INDIRECT_RW_RETRIES; retry++) { > + tmp = sdio_readb(rtwsdio->sdio_func, reg_cfg + 2, ret); > + if (!ret && tmp & BIT(4)) 'ret' is pointer, do you need '*' ? if (!*ret && tmp & BIT(4)) As I look into sdio_readb(), it use 'int *err_ret' as arugment. Would you like to change ' int *ret' to 'int *err_ret'? It could help to misunderstand. > + break; > + } > + > + if (*ret) > + return 0; > + > + return sdio_readb(rtwsdio->sdio_func, reg_data, ret); > +} > + [...] > + > +static void rtw_sdio_rx_aggregation(struct rtw_dev *rtwdev, bool enable) > +{ > + u8 size, timeout; > + > + if (enable) { > + if (rtwdev->chip->id == RTW_CHIP_TYPE_8822C) { > + size = 0xff; > + timeout = 0x20; > + } else { > + size = 0x6; > + timeout = 0x6; > + } > + > + /* Make the firmware honor the size limit configured below */ > + rtw_write32_set(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); > + > + rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); > + > + rtw_write16(rtwdev, REG_RXDMA_AGG_PG_TH, size | > + (timeout << BIT_SHIFT_DMA_AGG_TO_V1)); BIT_RXDMA_AGG_PG_TH GENMASK(7, 0) // for size BIT_DMA_AGG_TO_V1 GENMASK(15, 8) // for timeout > + > + rtw_write8_set(rtwdev, REG_RXDMA_MODE, BIT_DMA_MODE); > + } else { > + rtw_write32_clr(rtwdev, REG_RXDMA_AGG_PG_TH, BIT_EN_PRE_CALC); > + rtw_write8_clr(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_AGG_EN); > + rtw_write8_clr(rtwdev, REG_RXDMA_MODE, BIT_DMA_MODE); > + } > +} > + > +static void rtw_sdio_enable_interrupt(struct rtw_dev *rtwdev) > +{ > + struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv; > + > + rtw_write32(rtwdev, REG_SDIO_HIMR, rtwsdio->irq_mask); > +} > + > +static void rtw_sdio_disable_interrupt(struct rtw_dev *rtwdev) > +{ > + rtw_write32(rtwdev, REG_SDIO_HIMR, 0x0); > +} > + > +static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb, > + u8 queue) > +{ > + switch (queue) { > + case RTW_TX_QUEUE_BCN: > + return TX_DESC_QSEL_BEACON; > + case RTW_TX_QUEUE_H2C: > + return TX_DESC_QSEL_H2C; > + case RTW_TX_QUEUE_MGMT: > + if (rtw_chip_wcpu_11n(rtwdev)) > + return TX_DESC_QSEL_HIGH; > + else > + return TX_DESC_QSEL_MGMT; > + case RTW_TX_QUEUE_HI0: > + return TX_DESC_QSEL_HIGH; > + default: > + return skb->priority; > + } > +}; no need ';' [...] > + > +static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev) > +{ > + u32 rx_len; > + > + while (true) { add a limit to prevent infinite loop. > + if (rtw_chip_wcpu_11n(rtwdev)) > + rx_len = rtw_read16(rtwdev, REG_SDIO_RX0_REQ_LEN); > + else > + rx_len = rtw_read32(rtwdev, REG_SDIO_RX0_REQ_LEN); > + > + if (!rx_len) > + break; > + > + rtw_sdio_rxfifo_recv(rtwdev, rx_len); > + } > +} > + [...] > + > +static void rtw_sdio_process_tx_queue(struct rtw_dev *rtwdev, > + enum rtw_tx_queue_type queue) > +{ > + struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv; > + struct sk_buff *skb; > + int ret; > + > + while (true) { Can we have a limit? > + skb = skb_dequeue(&rtwsdio->tx_queue[queue]); > + if (!skb) > + break; > + > + ret = rtw_sdio_write_port(rtwdev, skb, queue); > + if (ret) { > + skb_queue_head(&rtwsdio->tx_queue[queue], skb); > + break; > + } > + > + if (queue <= RTW_TX_QUEUE_VO) > + rtw_sdio_indicate_tx_status(rtwdev, skb); > + else > + dev_kfree_skb_any(skb); > + } > +} > + [...]