Larry Finger <Larry.Finger@xxxxxxxxxxxx> writes:
> From: Ping-Ke Shih <pkshih@xxxxxxxxxxx>
>
> 1. Both 32-bit and 64-bit use the same TX/RX buffer desc layout
> 2. Extend set_desc() and get_desc() to set and get 64-bit address
> 3. Remove directive DMA_IS_64BIT
> 4. Add module parameter to turn on 64-bit dma
>
> Signed-off-by: Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> Signed-off-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> Cc: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx>
> Cc: Birming Chiu <birming@xxxxxxxxxxx>
> Cc: Shaofu <shaofu@xxxxxxxxxxx>
> Cc: Steven Ting <steventing@xxxxxxxxxxx>
I applied this already but I still want to give few general remarks
about frequent problems with rtlwifi patches:
> @@ -691,9 +691,10 @@ static int _rtl_pci_init_one_rxdesc(struct ieee80211_hw *hw,
> return 0;
> rtlpci->rx_ring[rxring_idx].rx_buf[desc_idx] = skb;
> if (rtlpriv->use_new_trx_flow) {
> + /* skb->cb may be 64 bit address */
> rtlpriv->cfg->ops->set_desc(hw, (u8 *)entry, false,
> HW_DESC_RX_PREPARE,
> - (u8 *)&bufferaddress);
> + (u8 *)(dma_addr_t *)skb->cb);
What I see a lot are these very questionable looking use of casts like
here. Casting should be also avoided as much as possible and used only
when it's absolutely necessary. If you have to do (u8) or (u8 *) almost
in every second line something is very wrong.
> +static void platform_enable_dma64(struct pci_dev *pdev, bool dma64)
> +{
> + u8 value;
> +
> + pci_read_config_byte(pdev, 0x719, &value);
> +
> + /* 0x719 Bit5 is DMA64 bit fetch. */
> + if (dma64)
> + value |= BIT(5);
> + else
> + value &= ~BIT(5);
> +
> + pci_write_config_byte(pdev, 0x719, value);
> +}
Another gripe I have is about the use of magic values. There always
should be a define/enum with a descriptive name for register addresses
and values.
--
Kalle Valo
