RE: [PATCH v2 08/11] wifi: rtw88: sdio: set up RX aggregation and interrupts for RTL8723BS

[Ping-Ke Shih <pkshih@realtek.com>]

luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
> From: Luka Gejak <luka.gejak@linux.dev>
> 
> Enable the existing RX aggregation setup for this chip and select the
> larger DMA aggregation mode it needs. The RTL8723BS does not raise
> CPWM1, so leave that source out of its interrupt mask, and set the SDIO
> TX control bit the vendor driver uses to have transfers always
> recognised.
> 
> The chip also keeps raising the interrupt after resume if undefined
> status bits are written back when acknowledging, so acknowledge only the
> defined and unmasked bits. That is scoped to this chip; the other SDIO
> parts keep writing the status word back unchanged.
> 
> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> ---
>  drivers/net/wireless/realtek/rtw88/sdio.c | 28 ++++++++++++++++++++++-
>  drivers/net/wireless/realtek/rtw88/sdio.h |  9 ++++++++
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index adf0b509e2cc..37d1d58fa55e 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -844,7 +844,11 @@ static void rtw_sdio_init(struct rtw_dev *rtwdev)
>  {
>         struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> 
> -       rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST | REG_SDIO_HIMR_CPWM1;
> +       if (rtw_is_8723bs(rtwdev))
> +               rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST;
> +       else
> +               rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST |
> +                                      REG_SDIO_HIMR_CPWM1;

align the REG_*

>  }
> 
>  static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
> @@ -852,6 +856,7 @@ static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
>         u8 size, timeout;
> 
>         switch (rtwdev->chip->id) {
> +       case RTW_CHIP_TYPE_8723B:
>         case RTW_CHIP_TYPE_8703B:
>         case RTW_CHIP_TYPE_8821A:
>         case RTW_CHIP_TYPE_8812A:
> @@ -879,6 +884,8 @@ static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
>                     FIELD_PREP(BIT_DMA_AGG_TO_V1, timeout));
> 
>         rtw_write8_set(rtwdev, REG_RXDMA_MODE, BIT_DMA_MODE);
> +       if (rtw_is_8723bs(rtwdev))
> +               rtw_write8_set(rtwdev, REG_RXDMA_MODE, 3 << 2);

I think you are setting burst count?  rtw_write8_mask()?

#define BIT_DMA_BURST_CNT	GENMASK(3, 2)

>  }
> 
>  static void rtw_sdio_enable_interrupt(struct rtw_dev *rtwdev)
> @@ -947,9 +954,19 @@ static void rtw_sdio_8723bs_check_rqpn(struct rtw_dev *rtwdev)
> 
>  static int rtw_sdio_start(struct rtw_dev *rtwdev)
>  {
> +       u32 clear;
> +
>         rtw_sdio_8723bs_check_rqpn(rtwdev);
>         rtw_sdio_init_free_txpg(rtwdev);
>         rtw_sdio_enable_rx_aggregation(rtwdev);
> +
> +       if (rtw_is_8723bs(rtwdev)) {
> +               clear = rtw_read32(rtwdev, REG_SDIO_HISR) &
> +                       RTW_SDIO_HISR_CLEAR_MASK;
> +               if (clear)
> +                       rtw_write32(rtwdev, REG_SDIO_HISR, clear);
> +       }
> +
>         rtw_sdio_enable_interrupt(rtwdev);
> 
>         return 0;
> @@ -1029,6 +1046,8 @@ static void rtw_sdio_interface_cfg(struct rtw_dev *rtwdev)
> 
>         val = rtw_read32(rtwdev, REG_SDIO_TX_CTRL);
>         val &= 0xfff8;
> +       if (rtw_is_8723bs(rtwdev))
> +               val |= BIT_SDIO_TX_CTRL_ALWAYS_RECOGNIZE;
>         rtw_write32(rtwdev, REG_SDIO_TX_CTRL, val);
>  }
> 
> @@ -1293,6 +1312,13 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
>                 rtw_sdio_rx_isr(rtwdev);
>         }
> 
> +       /* RTL8723BS keeps raising the interrupt after resume if undefined
> +        * status bits are written back, so acknowledge only the bits that are
> +        * both defined and unmasked. Other chips keep the existing behaviour.
> +        */

comment style

> +       if (rtw_is_8723bs(rtwdev))
> +               hisr &= rtwsdio->irq_mask & RTW_SDIO_HISR_CLEAR_MASK;
> +
>         rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
> 
>         rtwsdio->irq_thread = NULL;

[...]