[PATCH 0/5] qtnfmac: pcie datapath updates and fixes

[PATCH 0/5] qtnfmac: pcie datapath updates and fixes

[Sergey Matyukevich]

Hello Kalle, Igor, and all

This patches introduces further updates and fixes
for pcie datapath in qtnfmac driver:

- implement 64-bit dma for hosts enabling CONFIG_ARCH_DMA_ADDR_T_64BIT
- sanity check for module params: circ_buf queues should be power-of-2
- misc cleanup according to previous reviews

Sergey Matyukevich (5):

qtnfmac: drop -D__CHECK_ENDIAN from cflags
qtnfmac: module param sanity check
qtnfmac: modify qtnf_map_bar not to return NULL
qtnfmac: fix free_xfer_buffer cleanup
qtnfmac: implement 64-bit dma support

Makefile		| 4 -
pearl/pcie.c		| 101 +++++++++++++++++++++++++++++++++++++++---------
pearl/pcie_ipc.h	| 10 +---
pearl/pcie_regs_pearl.h | 1
4 files changed, 88 insertions(+), 28 deletions(-)

[PATCH 1/5] qtnfmac: drop -D__CHECK_ENDIAN from cflags

[Sergey Matyukevich]

Flag -D__CHECK_ENDIAN was wrong: it should be -D__CHECK_ENDIAN__ instead.
However now this flag is enabled by default, so it can be removed.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/Makefile b/drivers/net/wireless/quantenna/qtnfmac/Makefile
index 0d618e5e5f5b..f236b7dc2be3 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/Makefile
+++ b/drivers/net/wireless/quantenna/qtnfmac/Makefile
@@ -25,7 +25,3 @@ qtnfmac_pearl_pcie-objs += \
 	pearl/pcie.o
 
 qtnfmac_pearl_pcie-$(CONFIG_DEBUG_FS) += debug.o
-
-#
-
-ccflags-y += -D__CHECK_ENDIAN
-- 
2.11.0

Re: [1/5] qtnfmac: drop -D__CHECK_ENDIAN from cflags

[Kalle Valo]

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> wrote:

> Flag -D__CHECK_ENDIAN was wrong: it should be -D__CHECK_ENDIAN__ instead.
> However now this flag is enabled by default, so it can be removed.
> 
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

5 patches applied to wireless-drivers-next.git, thanks.

57b18a75d90e qtnfmac: drop -D__CHECK_ENDIAN from cflags
97f38011451f qtnfmac: module param sanity check
bab5dac73c08 qtnfmac: modify qtnf_map_bar not to return NULL
b00edea3ed5f qtnfmac: fix free_xfer_buffer cleanup
f31039d4aea9 qtnfmac: implement 64-bit dma support

-- 
https://patchwork.kernel.org/patch/9927237/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/5] qtnfmac: module param sanity check

[Sergey Matyukevich]

Linux built-in circ_buf implementation assumes that that the
circular buffer length is a power of 2. Make sure that
rx and tx descriptor queue lengths are power-of-2.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index cd2f2b667643..fd552d64f943 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -26,6 +26,7 @@
 #include <linux/crc32.h>
 #include <linux/spinlock.h>
 #include <linux/circ_buf.h>
+#include <linux/log2.h>
 
 #include "qtn_hw_ids.h"
 #include "pcie_bus_priv.h"
@@ -39,11 +40,11 @@ MODULE_PARM_DESC(use_msi, "set 0 to use legacy interrupt");
 
 static unsigned int tx_bd_size_param = 32;
 module_param(tx_bd_size_param, uint, 0644);
-MODULE_PARM_DESC(tx_bd_size_param, "Tx descriptors queue size");
+MODULE_PARM_DESC(tx_bd_size_param, "Tx descriptors queue size, power of two");
 
 static unsigned int rx_bd_size_param = 256;
 module_param(rx_bd_size_param, uint, 0644);
-MODULE_PARM_DESC(rx_bd_size_param, "Rx descriptors queue size");
+MODULE_PARM_DESC(rx_bd_size_param, "Rx descriptors queue size, power of two");
 
 static u8 flashboot = 1;
 module_param(flashboot, byte, 0644);
@@ -509,6 +510,18 @@ static int qtnf_pcie_init_xfer(struct qtnf_pcie_bus_priv *priv)
 	priv->rx_bd_w_index = 0;
 	priv->rx_bd_r_index = 0;
 
+	if (!priv->tx_bd_num || !is_power_of_2(priv->tx_bd_num)) {
+		pr_err("tx_bd_size_param %u is not power of two\n",
+		       priv->tx_bd_num);
+		return -EINVAL;
+	}
+
+	if (!priv->rx_bd_num || !is_power_of_2(priv->rx_bd_num)) {
+		pr_err("rx_bd_size_param %u is not power of two\n",
+		       priv->rx_bd_num);
+		return -EINVAL;
+	}
+
 	ret = alloc_skb_array(priv);
 	if (ret) {
 		pr_err("failed to allocate skb array\n");
-- 
2.11.0

[PATCH 3/5] qtnfmac: modify qtnf_map_bar not to return NULL

[Sergey Matyukevich]

NULL is not a special type of success here but a error pointer.
So it makes sense to check against NULL in qtnf_map_bar
and return error code.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index fd552d64f943..bfbcd0bf75bf 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -184,8 +184,10 @@ static void __iomem *qtnf_map_bar(struct qtnf_pcie_bus_priv *priv, u8 index)
 		return IOMEM_ERR_PTR(ret);
 
 	busaddr = pci_resource_start(priv->pdev, index);
-	vaddr = pcim_iomap_table(priv->pdev)[index];
 	len = pci_resource_len(priv->pdev, index);
+	vaddr = pcim_iomap_table(priv->pdev)[index];
+	if (!vaddr)
+		return IOMEM_ERR_PTR(-ENOMEM);
 
 	pr_debug("BAR%u vaddr=0x%p busaddr=%pad len=%u\n",
 		 index, vaddr, &busaddr, (int)len);
@@ -248,19 +250,19 @@ static int qtnf_pcie_init_memory(struct qtnf_pcie_bus_priv *priv)
 	int ret = -ENOMEM;
 
 	priv->sysctl_bar = qtnf_map_bar(priv, QTN_SYSCTL_BAR);
-	if (IS_ERR_OR_NULL(priv->sysctl_bar)) {
+	if (IS_ERR(priv->sysctl_bar)) {
 		pr_err("failed to map BAR%u\n", QTN_SYSCTL_BAR);
 		return ret;
 	}
 
 	priv->dmareg_bar = qtnf_map_bar(priv, QTN_DMA_BAR);
-	if (IS_ERR_OR_NULL(priv->dmareg_bar)) {
+	if (IS_ERR(priv->dmareg_bar)) {
 		pr_err("failed to map BAR%u\n", QTN_DMA_BAR);
 		return ret;
 	}
 
 	priv->epmem_bar = qtnf_map_bar(priv, QTN_SHMEM_BAR);
-	if (IS_ERR_OR_NULL(priv->epmem_bar)) {
+	if (IS_ERR(priv->epmem_bar)) {
 		pr_err("failed to map BAR%u\n", QTN_SHMEM_BAR);
 		return ret;
 	}
-- 
2.11.0

Re: [PATCH 3/5] qtnfmac: modify qtnf_map_bar not to return NULL

[Igor Mitsyanko]

On 08/29/2017 05:16 AM, Sergey Matyukevich wrote:
> NULL is not a special type of success here but a error pointer.
> So it makes sense to check against NULL in qtnf_map_bar
> and return error code.
> 
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> ---

On a first glance not immediately obvious what is logically changed 
here, is it so that pr_debug() would not print NULL pointer?

Re: [PATCH 3/5] qtnfmac: modify qtnf_map_bar not to return NULL

[Sergey Matyukevich]

On Tue, Aug 29, 2017 at 07:13:35PM -0700, Igor Mitsyanko wrote:
> On 08/29/2017 05:16 AM, Sergey Matyukevich wrote:
> > NULL is not a special type of success here but a error pointer.
> > So it makes sense to check against NULL in qtnf_map_bar
> > and return error code.
> > 
> > Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> > ---
> 
> On a first glance not immediately obvious what is logically changed here, is
> it so that pr_debug() would not print NULL pointer?

There is no actual bug here: all the mappings and error checks are in place.
This is more about coding style problem: when function return both NULL and
error pointeres, then the NULL is supposed to be a special type of success
return. In this particular case NULL is just yet another failure.

Re: [PATCH 3/5] qtnfmac: modify qtnf_map_bar not to return NULL

[Igor Mitsyanko]

On 08/29/2017 05:16 AM, Sergey Matyukevich wrote:
> NULL is not a special type of success here but a error pointer.
> So it makes sense to check against NULL in qtnf_map_bar
> and return error code.
> 
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> ---

On a first glance not immediately obvious what is logically changed 
here, is it so that pr_debug() would not print NULL pointer?

[PATCH 4/5] qtnfmac: fix free_xfer_buffer cleanup

[Sergey Matyukevich]

Check if skb tracking arrays has been already allocated. This additional
check handles the case when init partially failed.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index bfbcd0bf75bf..2921d8069bf2 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -483,7 +483,7 @@ static void free_xfer_buffers(void *data)
 
 	/* free rx buffers */
 	for (i = 0; i < priv->rx_bd_num; i++) {
-		if (priv->rx_skb[i]) {
+		if (priv->rx_skb && priv->rx_skb[i]) {
 			rxbd = &priv->rx_bd_vbase[i];
 			paddr = QTN_HOST_ADDR(le32_to_cpu(rxbd->addr_h),
 					      le32_to_cpu(rxbd->addr));
@@ -496,7 +496,7 @@ static void free_xfer_buffers(void *data)
 
 	/* free tx buffers */
 	for (i = 0; i < priv->tx_bd_num; i++) {
-		if (priv->tx_skb[i]) {
+		if (priv->tx_skb && priv->tx_skb[i]) {
 			dev_kfree_skb_any(priv->tx_skb[i]);
 			priv->tx_skb[i] = NULL;
 		}
-- 
2.11.0

[PATCH 5/5] qtnfmac: implement 64-bit dma support

[Sergey Matyukevich]

Use 64-bit dma for hosts with CONFIG_ARCH_DMA_ADDR_T_64BIT enabled.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 .../net/wireless/quantenna/qtnfmac/pearl/pcie.c    | 70 ++++++++++++++++++----
 .../wireless/quantenna/qtnfmac/pearl/pcie_ipc.h    | 10 ++--
 .../quantenna/qtnfmac/pearl/pcie_regs_pearl.h      |  1 +
 3 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index 2921d8069bf2..502e72b7cdcc 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -403,10 +403,12 @@ static int alloc_bd_table(struct qtnf_pcie_bus_priv *priv)
 	priv->rx_bd_vbase = vaddr;
 	priv->rx_bd_pbase = paddr;
 
-	writel(QTN_HOST_LO32(paddr),
-	       PCIE_HDP_TX_HOST_Q_BASE_L(priv->pcie_reg_base));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	writel(QTN_HOST_HI32(paddr),
 	       PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));
+#endif
+	writel(QTN_HOST_LO32(paddr),
+	       PCIE_HDP_TX_HOST_Q_BASE_L(priv->pcie_reg_base));
 	writel(priv->rx_bd_num | (sizeof(struct qtnf_rx_bd)) << 16,
 	       PCIE_HDP_TX_HOST_Q_SZ_CTRL(priv->pcie_reg_base));
 
@@ -447,8 +449,10 @@ static int skb2rbd_attach(struct qtnf_pcie_bus_priv *priv, u16 index)
 	/* sync up all descriptor updates */
 	wmb();
 
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	writel(QTN_HOST_HI32(paddr),
 	       PCIE_HDP_HHBM_BUF_PTR_H(priv->pcie_reg_base));
+#endif
 	writel(QTN_HOST_LO32(paddr),
 	       PCIE_HDP_HHBM_BUF_PTR(priv->pcie_reg_base));
 
@@ -503,9 +507,28 @@ static void free_xfer_buffers(void *data)
 	}
 }
 
+static int qtnf_hhbm_init(struct qtnf_pcie_bus_priv *priv)
+{
+	u32 val;
+
+	val = readl(PCIE_HHBM_CONFIG(priv->pcie_reg_base));
+	val |= HHBM_CONFIG_SOFT_RESET;
+	writel(val, PCIE_HHBM_CONFIG(priv->pcie_reg_base));
+	usleep_range(50, 100);
+	val &= ~HHBM_CONFIG_SOFT_RESET;
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	val |= HHBM_64BIT;
+#endif
+	writel(val, PCIE_HHBM_CONFIG(priv->pcie_reg_base));
+	writel(priv->rx_bd_num, PCIE_HHBM_Q_LIMIT_REG(priv->pcie_reg_base));
+
+	return 0;
+}
+
 static int qtnf_pcie_init_xfer(struct qtnf_pcie_bus_priv *priv)
 {
 	int ret;
+	u32 val;
 
 	priv->tx_bd_num = tx_bd_size_param;
 	priv->rx_bd_num = rx_bd_size_param;
@@ -518,12 +541,32 @@ static int qtnf_pcie_init_xfer(struct qtnf_pcie_bus_priv *priv)
 		return -EINVAL;
 	}
 
+	val = priv->tx_bd_num * sizeof(struct qtnf_tx_bd);
+	if (val > PCIE_HHBM_MAX_SIZE) {
+		pr_err("tx_bd_size_param %u is too large\n",
+		       priv->tx_bd_num);
+		return -EINVAL;
+	}
+
 	if (!priv->rx_bd_num || !is_power_of_2(priv->rx_bd_num)) {
 		pr_err("rx_bd_size_param %u is not power of two\n",
 		       priv->rx_bd_num);
 		return -EINVAL;
 	}
 
+	val = priv->rx_bd_num * sizeof(dma_addr_t);
+	if (val > PCIE_HHBM_MAX_SIZE) {
+		pr_err("rx_bd_size_param %u is too large\n",
+		       priv->rx_bd_num);
+		return -EINVAL;
+	}
+
+	ret = qtnf_hhbm_init(priv);
+	if (ret) {
+		pr_err("failed to init h/w queues\n");
+		return ret;
+	}
+
 	ret = alloc_skb_array(priv);
 	if (ret) {
 		pr_err("failed to allocate skb array\n");
@@ -653,10 +696,13 @@ static int qtnf_pcie_data_tx(struct qtnf_bus *bus, struct sk_buff *skb)
 
 	/* write new TX descriptor to PCIE_RX_FIFO on EP */
 	txbd_paddr = priv->tx_bd_pbase + i * sizeof(struct qtnf_tx_bd);
-	writel(QTN_HOST_LO32(txbd_paddr),
-	       PCIE_HDP_HOST_WR_DESC0(priv->pcie_reg_base));
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	writel(QTN_HOST_HI32(txbd_paddr),
 	       PCIE_HDP_HOST_WR_DESC0_H(priv->pcie_reg_base));
+#endif
+	writel(QTN_HOST_LO32(txbd_paddr),
+	       PCIE_HDP_HOST_WR_DESC0(priv->pcie_reg_base));
 
 	if (++i >= priv->tx_bd_num)
 		i = 0;
@@ -1237,6 +1283,16 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		pr_debug("successful init of PCI device %x\n", pdev->device);
 	}
 
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+#else
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+#endif
+	if (ret) {
+		pr_err("PCIE DMA coherent mask init failed\n");
+		goto err_base;
+	}
+
 	pcim_pin_device(pdev);
 	pci_set_master(pdev);
 
@@ -1258,12 +1314,6 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_base;
 	}
 
-	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
-	if (ret) {
-		pr_err("PCIE DMA mask init failed\n");
-		goto err_base;
-	}
-
 	ret = devm_add_action(&pdev->dev, free_xfer_buffers, (void *)pcie_priv);
 	if (ret) {
 		pr_err("custom release callback init failed\n");
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h
index 667f5ec457e3..c5a4e46d26ef 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h
@@ -57,16 +57,14 @@
 	| PCIE_HDP_INT_EP_RXDMA		\
 	)
 
-#if BITS_PER_LONG == 64
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 #define QTN_HOST_HI32(a)	((u32)(((u64)a) >> 32))
 #define QTN_HOST_LO32(a)	((u32)(((u64)a) & 0xffffffffUL))
 #define QTN_HOST_ADDR(h, l)	((((u64)h) << 32) | ((u64)l))
-#elif BITS_PER_LONG == 32
+#else
 #define QTN_HOST_HI32(a)	0
 #define QTN_HOST_LO32(a)	((u32)(((u32)a) & 0xffffffffUL))
 #define QTN_HOST_ADDR(h, l)	((u32)l)
-#else
-#error Unexpected BITS_PER_LONG value
 #endif
 
 #define QTN_SYSCTL_BAR	0
@@ -76,7 +74,7 @@
 #define QTN_PCIE_BDA_VERSION		0x1002
 
 #define PCIE_BDA_NAMELEN		32
-#define PCIE_HHBM_MAX_SIZE		512
+#define PCIE_HHBM_MAX_SIZE		2048
 
 #define SKB_BUF_SIZE		2048
 
@@ -113,7 +111,7 @@ struct qtnf_pcie_bda {
 	__le32 bda_flashsz;
 	u8 bda_boardname[PCIE_BDA_NAMELEN];
 	__le32 bda_rc_msi_enabled;
-	__le32 bda_hhbm_list[PCIE_HHBM_MAX_SIZE];
+	u8 bda_hhbm_list[PCIE_HHBM_MAX_SIZE];
 	__le32 bda_dsbw_start_index;
 	__le32 bda_dsbw_end_index;
 	__le32 bda_dsbw_total_bytes;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h
index 69696f118769..5b48b425fa7f 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h
@@ -109,6 +109,7 @@
 #define HHBM_WR_REQ				(BIT(0))
 #define HHBM_RD_REQ				(BIT(1))
 #define HHBM_DONE				(BIT(31))
+#define HHBM_64BIT				(BIT(10))
 
 /* offsets for dual PCIE */
 #define PCIE_PORT_LINK_CTL(base)		((base) + 0x0710)
-- 
2.11.0

Re: [PATCH 5/5] qtnfmac: implement 64-bit dma support

[Kalle Valo]

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> Use 64-bit dma for hosts with CONFIG_ARCH_DMA_ADDR_T_64BIT enabled.
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> ---
>  .../net/wireless/quantenna/qtnfmac/pearl/pcie.c    | 70 ++++++++++++++++++----
>  .../wireless/quantenna/qtnfmac/pearl/pcie_ipc.h    | 10 ++--
>  .../quantenna/qtnfmac/pearl/pcie_regs_pearl.h      |  1 +
>  3 files changed, 65 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
> index 2921d8069bf2..502e72b7cdcc 100644
> --- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
> +++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
> @@ -403,10 +403,12 @@ static int alloc_bd_table(struct qtnf_pcie_bus_priv *priv)
>  	priv->rx_bd_vbase = vaddr;
>  	priv->rx_bd_pbase = paddr;
>  
> -	writel(QTN_HOST_LO32(paddr),
> -	       PCIE_HDP_TX_HOST_Q_BASE_L(priv->pcie_reg_base));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>  	writel(QTN_HOST_HI32(paddr),
>  	       PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));
> +#endif

Personally I detest ifdefs and try to write code like this using
IS_ENABLED():

if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
  	writel(QTN_HOST_HI32(paddr),
  	       PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));

But up to you which style you prefer.

-- 
Kalle Valo

Re: [PATCH 5/5] qtnfmac: implement 64-bit dma support

[Sergey Matyukevich]

Hello Kalle,

> > -     writel(QTN_HOST_LO32(paddr),
> > -            PCIE_HDP_TX_HOST_Q_BASE_L(priv->pcie_reg_base));
> > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> >       writel(QTN_HOST_HI32(paddr),
> >              PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));
> > +#endif
> 
> Personally I detest ifdefs and try to write code like this using
> IS_ENABLED():
> 
> if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
>         writel(QTN_HOST_HI32(paddr),
>                PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));
> 
> But up to you which style you prefer.

I agree that this way it looks better. But I am using the same ifdef in header
to define QTN_HOST_* macros. So in this particular case I would prefer to
keep the same notation in both source and header files.

Thanks,
Sergey

Re: [PATCH 5/5] qtnfmac: implement 64-bit dma support

[Kalle Valo]

Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:

> Hello Kalle,
>
>> > -     writel(QTN_HOST_LO32(paddr),
>> > -            PCIE_HDP_TX_HOST_Q_BASE_L(priv->pcie_reg_base));
>> > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> >       writel(QTN_HOST_HI32(paddr),
>> >              PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));
>> > +#endif
>> 
>> Personally I detest ifdefs and try to write code like this using
>> IS_ENABLED():
>> 
>> if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
>>         writel(QTN_HOST_HI32(paddr),
>>                PCIE_HDP_TX_HOST_Q_BASE_H(priv->pcie_reg_base));
>> 
>> But up to you which style you prefer.
>
> I agree that this way it looks better. But I am using the same ifdef in header
> to define QTN_HOST_* macros. So in this particular case I would prefer to
> keep the same notation in both source and header files.

Ok. I'll try to get these to 4.14 still.

-- 
Kalle Valo