diff mbox series

[RFC,V2,3/6] net: bcmgenet: use optional max DMA burst size property

Message ID 1572702093-18261-4-git-send-email-wahrenst@gmx.net (mailing list archive)
State New, archived
Headers show
Series ARM: Enable GENET support for RPi 4 | expand

Commit Message

Stefan Wahren Nov. 2, 2019, 1:41 p.m. UTC 
From: Matthias Brugger <mbrugger@suse.com>

Depending on the HW, the maximal usable DMA burst size can vary.
If not set accordingly a timeout in the transmit queue happens and no
package can be sent. Read to optional max-burst-sz property, if not
present, fallback to the standard value.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++++++--
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

--
2.7.4

Comments

Florian Fainelli Nov. 2, 2019, 7:34 p.m. UTC | #1 
On 11/2/2019 6:41 AM, Stefan Wahren wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> Depending on the HW, the maximal usable DMA burst size can vary.
> If not set accordingly a timeout in the transmit queue happens and no
> package can be sent. Read to optional max-burst-sz property, if not
> present, fallback to the standard value.
> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>

Missing your Signed-off-by here since you are carrying this patch from
Matthias.

> ---

[snip]

> +	if (dn) {
> +		of_property_read_u32(dn, "dma-burst-sz",
> +				     &priv->dma_max_burst_length);
> +	} else {
> +		priv->dma_max_burst_length = DMA_MAX_BURST_LENGTH;
> +	}

I would maintain the previous position provided on Matthias' patch
series, which is the integration of the GENETv5 hardware block in 2711
is done in a way that is different enough (due to the SCB/AXI bridge)
that a separate compatibility string would be in order. Once you that
defined that "brcm,bcm2711-genet-v5" compatibility string defined, you
can derive the DMA burst size off of it.

If adding a compatibility string is not practical because of the
downstream DTBs, then can we at least fix this patch in two ways:

- define the property in the binding document
- spell out the property in full names: max-dma-burst-size so as to
reflect what it does

Thanks!
Stefan Wahren Nov. 3, 2019, 9:48 a.m. UTC | #2 
Hi Florian,

Am 02.11.19 um 20:34 schrieb Florian Fainelli:
> On 11/2/2019 6:41 AM, Stefan Wahren wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> Depending on the HW, the maximal usable DMA burst size can vary.
>> If not set accordingly a timeout in the transmit queue happens and no
>> package can be sent. Read to optional max-burst-sz property, if not
>> present, fallback to the standard value.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Missing your Signed-off-by here since you are carrying this patch from
> Matthias.
oops
>
>> ---
> [snip]
>
>> +	if (dn) {
>> +		of_property_read_u32(dn, "dma-burst-sz",
>> +				     &priv->dma_max_burst_length);
>> +	} else {
>> +		priv->dma_max_burst_length = DMA_MAX_BURST_LENGTH;
>> +	}
> I would maintain the previous position provided on Matthias' patch
> series, which is the integration of the GENETv5 hardware block in 2711
> is done in a way that is different enough (due to the SCB/AXI bridge)
> that a separate compatibility string would be in order. Once you that
> defined that "brcm,bcm2711-genet-v5" compatibility string defined, you
> can derive the DMA burst size off of it.
this is what i meant with didn't address all your comments. I'm fine
with your suggestion, but too lazy to integrate it in this patch series.
I assumed Matthias already take care of it.
> If adding a compatibility string is not practical because of the
> downstream DTBs, then can we at least fix this patch in two ways:
>
> - define the property in the binding document
> - spell out the property in full names: max-dma-burst-size so as to
> reflect what it does

In case of incompatibilities with the downstream DTB, i will take care
of it in the downstream tree like last time.

Regards
Stefan

>
> Thanks!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index ac554a6..8d0093b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2578,7 +2578,8 @@  static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
 	}

 	/* Init rDma */
-	bcmgenet_rdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+	bcmgenet_rdma_writel(priv, priv->dma_max_burst_length,
+			     DMA_SCB_BURST_SIZE);

 	/* Initialize Rx queues */
 	ret = bcmgenet_init_rx_queues(priv->dev);
@@ -2591,7 +2592,8 @@  static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
 	}

 	/* Init tDma */
-	bcmgenet_tdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+	bcmgenet_tdma_writel(priv, priv->dma_max_burst_length,
+			     DMA_SCB_BURST_SIZE);

 	/* Initialize Tx queues */
 	bcmgenet_init_tx_queues(priv->dev);
@@ -3536,6 +3538,13 @@  static int bcmgenet_probe(struct platform_device *pdev)

 	clk_prepare_enable(priv->clk);

+	if (dn) {
+		of_property_read_u32(dn, "dma-burst-sz",
+				     &priv->dma_max_burst_length);
+	} else {
+		priv->dma_max_burst_length = DMA_MAX_BURST_LENGTH;
+	}
+
 	bcmgenet_set_hw_params(priv);

 	/* Mii wait queue */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 7fbf573..22cde8c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -664,6 +664,7 @@  struct bcmgenet_priv {
 	bool crc_fwd_en;

 	unsigned int dma_rx_chk_bit;
+	unsigned int dma_max_burst_length;

 	u32 msg_enable;