diff mbox series

[10/11] net: macb: Add support for RP1's MACB variant

Message ID 775000dfb3a35bc691010072942253cb022750e1.1724159867.git.andrea.porta@suse.com (mailing list archive)
State Not Applicable, archived
Headers show
Series Add support for RaspberryPi RP1 PCI device using a DT overlay | expand

Commit Message

Andrea della Porta Aug. 20, 2024, 2:36 p.m. UTC
RaspberryPi RP1 contains Cadence's MACB core. Implement the
changes to be able to operate the customization in the RP1.

Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
 drivers/net/ethernet/cadence/macb.h      |  25 ++++
 drivers/net/ethernet/cadence/macb_main.c | 152 ++++++++++++++++++++++-
 2 files changed, 175 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Aug. 20, 2024, 3:13 p.m. UTC | #1
> +static unsigned int txdelay = 35;
> +module_param(txdelay, uint, 0644);

Networking does not like module parameters.

This is also unused in this patch! So i suggest you just delete it.

> +
>  /* This structure is only used for MACB on SiFive FU540 devices */
>  struct sifive_fu540_macb_mgmt {
>  	void __iomem *reg;
> @@ -334,7 +337,7 @@ static int macb_mdio_wait_for_idle(struct macb *bp)
>  	u32 val;
>  
>  	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
> -				  1, MACB_MDIO_TIMEOUT);
> +				  100, MACB_MDIO_TIMEOUT);
>  }
  
Please take this patch out of the series, and break it up. This is one
patch, with a good explanation why you need 1->100.

>  static int macb_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
> @@ -493,6 +496,19 @@ static int macb_mdio_write_c45(struct mii_bus *bus, int mii_id,
>  	return status;
>  }
>  
> +static int macb_mdio_reset(struct mii_bus *bus)
> +{
> +	struct macb *bp = bus->priv;
> +
> +	if (bp->phy_reset_gpio) {
> +		gpiod_set_value_cansleep(bp->phy_reset_gpio, 1);
> +		msleep(bp->phy_reset_ms);
> +		gpiod_set_value_cansleep(bp->phy_reset_gpio, 0);
> +	}
> +
> +	return 0;
> +}
> +
>  static void macb_init_buffers(struct macb *bp)
>  {
>  	struct macb_queue *queue;
> @@ -969,6 +985,7 @@ static int macb_mii_init(struct macb *bp)
>  	bp->mii_bus->write = &macb_mdio_write_c22;
>  	bp->mii_bus->read_c45 = &macb_mdio_read_c45;
>  	bp->mii_bus->write_c45 = &macb_mdio_write_c45;
> +	bp->mii_bus->reset = &macb_mdio_reset;

This is one patch.

>  	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
>  		 bp->pdev->name, bp->pdev->id);
>  	bp->mii_bus->priv = bp;
> @@ -1640,6 +1657,11 @@ static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
>  
>  		macb_init_rx_ring(queue);
>  		queue_writel(queue, RBQP, queue->rx_ring_dma);
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> +			macb_writel(bp, RBQPH,
> +				    upper_32_bits(queue->rx_ring_dma));
> +#endif

How does this affect a disto kernel? Do you actually need the #ifdef?
What does bp->hw_dma_cap contain when CONFIG_ARCH_DMA_ADDR_T_64BIT is
not defined?

Again, this should be a patch of its own, with a good commit message.

Interrupt coalescing should be a patch of its own, etc.

    Andrew

---
pw-bot: cr
Andrea della Porta Aug. 20, 2024, 6:31 p.m. UTC | #2
Hi Andrew,

On 17:13 Tue 20 Aug     , Andrew Lunn wrote:
> > +static unsigned int txdelay = 35;
> > +module_param(txdelay, uint, 0644);
> 
> Networking does not like module parameters.
> 
> This is also unused in this patch! So i suggest you just delete it.
> 
> > +
> >  /* This structure is only used for MACB on SiFive FU540 devices */
> >  struct sifive_fu540_macb_mgmt {
> >  	void __iomem *reg;
> > @@ -334,7 +337,7 @@ static int macb_mdio_wait_for_idle(struct macb *bp)
> >  	u32 val;
> >  
> >  	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
> > -				  1, MACB_MDIO_TIMEOUT);
> > +				  100, MACB_MDIO_TIMEOUT);
> >  }
>   
> Please take this patch out of the series, and break it up. This is one
> patch, with a good explanation why you need 1->100.
> 
> >  static int macb_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
> > @@ -493,6 +496,19 @@ static int macb_mdio_write_c45(struct mii_bus *bus, int mii_id,
> >  	return status;
> >  }
> >  
> > +static int macb_mdio_reset(struct mii_bus *bus)
> > +{
> > +	struct macb *bp = bus->priv;
> > +
> > +	if (bp->phy_reset_gpio) {
> > +		gpiod_set_value_cansleep(bp->phy_reset_gpio, 1);
> > +		msleep(bp->phy_reset_ms);
> > +		gpiod_set_value_cansleep(bp->phy_reset_gpio, 0);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static void macb_init_buffers(struct macb *bp)
> >  {
> >  	struct macb_queue *queue;
> > @@ -969,6 +985,7 @@ static int macb_mii_init(struct macb *bp)
> >  	bp->mii_bus->write = &macb_mdio_write_c22;
> >  	bp->mii_bus->read_c45 = &macb_mdio_read_c45;
> >  	bp->mii_bus->write_c45 = &macb_mdio_write_c45;
> > +	bp->mii_bus->reset = &macb_mdio_reset;
> 
> This is one patch.
> 
> >  	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> >  		 bp->pdev->name, bp->pdev->id);
> >  	bp->mii_bus->priv = bp;
> > @@ -1640,6 +1657,11 @@ static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
> >  
> >  		macb_init_rx_ring(queue);
> >  		queue_writel(queue, RBQP, queue->rx_ring_dma);
> > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> > +		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> > +			macb_writel(bp, RBQPH,
> > +				    upper_32_bits(queue->rx_ring_dma));
> > +#endif
> 
> How does this affect a disto kernel? Do you actually need the #ifdef?
> What does bp->hw_dma_cap contain when CONFIG_ARCH_DMA_ADDR_T_64BIT is
> not defined?
> 
> Again, this should be a patch of its own, with a good commit message.
> 
> Interrupt coalescing should be a patch of its own, etc.
> 
>     Andrew
>

Thanks for the feedback, I agree on all the observations. Please do note
however that, as mentioned in the cover letter, this patch is not intended
to be included upstream and is provided just as a quick way for anyone
interested in testing the RP1 functionality using the Ethernet MAC. 
As such, this patch has not been polished nor splitted into manageable bits.
Ii'm taknge note of your comments however and will come back to them in a
future patch that deals specifically with macb.

Many thanks,
Andrea
 
> ---
> pw-bot: cr
Krzysztof Kozlowski Aug. 21, 2024, 8:49 a.m. UTC | #3
On Tue, Aug 20, 2024 at 04:36:12PM +0200, Andrea della Porta wrote:
> RaspberryPi RP1 contains Cadence's MACB core. Implement the
> changes to be able to operate the customization in the RP1.
> 
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>


> @@ -5100,6 +5214,11 @@ static int macb_probe(struct platform_device *pdev)
>  			}
>  		}
>  	}
> +
> +	device_property_read_u8(&pdev->dev, "cdns,aw2w-max-pipe", &bp->aw2w_max_pipe);
> +	device_property_read_u8(&pdev->dev, "cdns,ar2r-max-pipe", &bp->ar2r_max_pipe);

Where are the bindings?

> +	bp->use_aw2b_fill = device_property_read_bool(&pdev->dev, "cdns,use-aw2b-fill");
> +
>  	spin_lock_init(&bp->lock);
>  
>  	/* setup capabilities */
> @@ -5155,6 +5274,21 @@ static int macb_probe(struct platform_device *pdev)
>  	else
>  		bp->phy_interface = interface;
>  
> +	/* optional PHY reset-related properties */
> +	bp->phy_reset_gpio = devm_gpiod_get_optional(&pdev->dev, "phy-reset",

Where is the binding?

> +						     GPIOD_OUT_LOW);
> +	if (IS_ERR(bp->phy_reset_gpio)) {
> +		dev_err(&pdev->dev, "Failed to obtain phy-reset gpio\n");
> +		err = PTR_ERR(bp->phy_reset_gpio);
> +		goto err_out_free_netdev;
> +	}
> +
> +	bp->phy_reset_ms = 10;
> +	of_property_read_u32(np, "phy-reset-duration", &bp->phy_reset_ms);

Where is the binding?

> +	/* A sane reset duration should not be longer than 1s */
> +	if (bp->phy_reset_ms > 1000)
> +		bp->phy_reset_ms = 1000;
> +
>  	/* IP specific init */
>  	err = init(pdev);

Best regards,
Krzysztof
Florian Fainelli Aug. 21, 2024, 5:01 p.m. UTC | #4
On 8/20/24 07:36, Andrea della Porta wrote:
> RaspberryPi RP1 contains Cadence's MACB core. Implement the
> changes to be able to operate the customization in the RP1.
> 
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>

You are doing a lot of things, all at once, and you should consider 
extracting your change into a smaller subset with bug fixes first:

- one commit which writes to the RBQPH the upper 32-bits of the RX ring 
DMA address, that looks like a bug fix

- one commit which retriggers a buffer read, even though that appears to 
be RP1 specific maybe, if not, then this is also a bug fix

- one commit that adds support for macb_shutdown() to kill DMA operations

- one commit which adds support for a configurable PHY reset line + 
delay specified in milli seconds

- one commit which adds support for controling the interrupt coalescing 
settings

And then you can add all of the RP1 specific bits like the AXI bridge 
configuration.

[snip]

> @@ -1228,6 +1246,7 @@ struct macb_queue {
>   	dma_addr_t		tx_ring_dma;
>   	struct work_struct	tx_error_task;
>   	bool			txubr_pending;
> +	bool			tx_pending;
>   	struct napi_struct	napi_tx;
>   
>   	dma_addr_t		rx_ring_dma;
> @@ -1293,9 +1312,15 @@ struct macb {
>   
>   	u32			caps;
>   	unsigned int		dma_burst_length;
> +	u8			aw2w_max_pipe;
> +	u8			ar2r_max_pipe;
> +	bool			use_aw2b_fill;
>   
>   	phy_interface_t		phy_interface;
>   
> +	struct gpio_desc	*phy_reset_gpio;
> +	int			phy_reset_ms;

The delay cannot be negative, so this needs to be unsigned int.

> +
>   	/* AT91RM9200 transmit queue (1 on wire + 1 queued) */
>   	struct macb_tx_skb	rm9200_txq[2];
>   	unsigned int		max_tx_length;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 11665be3a22c..5eb5be6c96fc 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -41,6 +41,9 @@
>   #include <linux/inetdevice.h>
>   #include "macb.h"
>   
> +static unsigned int txdelay = 35;
> +module_param(txdelay, uint, 0644);
> +
>   /* This structure is only used for MACB on SiFive FU540 devices */
>   struct sifive_fu540_macb_mgmt {
>   	void __iomem *reg;
> @@ -334,7 +337,7 @@ static int macb_mdio_wait_for_idle(struct macb *bp)
>   	u32 val;
>   
>   	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
> -				  1, MACB_MDIO_TIMEOUT);
> +				  100, MACB_MDIO_TIMEOUT);

Why do we need to increase how frequently we poll?
Andrea della Porta Aug. 26, 2024, 8:03 p.m. UTC | #5
Hi Florian,

On 10:01 Wed 21 Aug     , Florian Fainelli wrote:
> On 8/20/24 07:36, Andrea della Porta wrote:
> > RaspberryPi RP1 contains Cadence's MACB core. Implement the
> > changes to be able to operate the customization in the RP1.
> > 
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> 
> You are doing a lot of things, all at once, and you should consider
> extracting your change into a smaller subset with bug fixes first:
> 
> - one commit which writes to the RBQPH the upper 32-bits of the RX ring DMA
> address, that looks like a bug fix
> 
> - one commit which retriggers a buffer read, even though that appears to be
> RP1 specific maybe, if not, then this is also a bug fix
> 
> - one commit that adds support for macb_shutdown() to kill DMA operations
> 
> - one commit which adds support for a configurable PHY reset line + delay
> specified in milli seconds
> 
> - one commit which adds support for controling the interrupt coalescing
> settings
> 
> And then you can add all of the RP1 specific bits like the AXI bridge
> configuration.
> 
> [snip]
> 
> > @@ -1228,6 +1246,7 @@ struct macb_queue {
> >   	dma_addr_t		tx_ring_dma;
> >   	struct work_struct	tx_error_task;
> >   	bool			txubr_pending;
> > +	bool			tx_pending;
> >   	struct napi_struct	napi_tx;
> >   	dma_addr_t		rx_ring_dma;
> > @@ -1293,9 +1312,15 @@ struct macb {
> >   	u32			caps;
> >   	unsigned int		dma_burst_length;
> > +	u8			aw2w_max_pipe;
> > +	u8			ar2r_max_pipe;
> > +	bool			use_aw2b_fill;
> >   	phy_interface_t		phy_interface;
> > +	struct gpio_desc	*phy_reset_gpio;
> > +	int			phy_reset_ms;
> 
> The delay cannot be negative, so this needs to be unsigned int.
> 
> > +
> >   	/* AT91RM9200 transmit queue (1 on wire + 1 queued) */
> >   	struct macb_tx_skb	rm9200_txq[2];
> >   	unsigned int		max_tx_length;
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index 11665be3a22c..5eb5be6c96fc 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -41,6 +41,9 @@
> >   #include <linux/inetdevice.h>
> >   #include "macb.h"
> > +static unsigned int txdelay = 35;
> > +module_param(txdelay, uint, 0644);
> > +
> >   /* This structure is only used for MACB on SiFive FU540 devices */
> >   struct sifive_fu540_macb_mgmt {
> >   	void __iomem *reg;
> > @@ -334,7 +337,7 @@ static int macb_mdio_wait_for_idle(struct macb *bp)
> >   	u32 val;
> >   	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
> > -				  1, MACB_MDIO_TIMEOUT);
> > +				  100, MACB_MDIO_TIMEOUT);
> 
> Why do we need to increase how frequently we poll?

Thanks for your feedback, I will save all your precious suggestions for a future patch that
will enable the macb ethernet.
As stated in the cover letter, right now this specific patch is not intended to be upstreamed
as is but it's just here for testing purposes, hence its 'raw' state.
For sure the ethernet contained in RP1 will be one of the first device I will try to bring
upstream, so I'll apply your comments there. Maybe the next time I will also add a better
explanation about the state of a specific patch in the commit comment itself, and not only
in the cover letter, just to be more explicit.

Many thanks,
Andrea 

> -- 
> Florian
>
Andrea della Porta Aug. 30, 2024, 10:32 p.m. UTC | #6
Hi Krzysztof,

On 10:49 Wed 21 Aug     , Krzysztof Kozlowski wrote:
> On Tue, Aug 20, 2024 at 04:36:12PM +0200, Andrea della Porta wrote:
> > RaspberryPi RP1 contains Cadence's MACB core. Implement the
> > changes to be able to operate the customization in the RP1.
> > 
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> 
> 
> > @@ -5100,6 +5214,11 @@ static int macb_probe(struct platform_device *pdev)
> >  			}
> >  		}
> >  	}
> > +
> > +	device_property_read_u8(&pdev->dev, "cdns,aw2w-max-pipe", &bp->aw2w_max_pipe);
> > +	device_property_read_u8(&pdev->dev, "cdns,ar2r-max-pipe", &bp->ar2r_max_pipe);
> 
> Where are the bindings?

As stated in the cover letter, this patch (and the dtb patch #11 for macb) is completely
unpolished and intended only for a quick test of the ethernet peripheral underneath
the RP1.  As such, it's not intended to be upstreamed yet.
However, your feedback is really appreaciated and will be used in a future patch that
will deal with ethernet mac support.

> 
> > +	bp->use_aw2b_fill = device_property_read_bool(&pdev->dev, "cdns,use-aw2b-fill");
> > +
> >  	spin_lock_init(&bp->lock);
> >  
> >  	/* setup capabilities */
> > @@ -5155,6 +5274,21 @@ static int macb_probe(struct platform_device *pdev)
> >  	else
> >  		bp->phy_interface = interface;
> >  
> > +	/* optional PHY reset-related properties */
> > +	bp->phy_reset_gpio = devm_gpiod_get_optional(&pdev->dev, "phy-reset",
> 
> Where is the binding?

Ditto.

> 
> > +						     GPIOD_OUT_LOW);
> > +	if (IS_ERR(bp->phy_reset_gpio)) {
> > +		dev_err(&pdev->dev, "Failed to obtain phy-reset gpio\n");
> > +		err = PTR_ERR(bp->phy_reset_gpio);
> > +		goto err_out_free_netdev;
> > +	}
> > +
> > +	bp->phy_reset_ms = 10;
> > +	of_property_read_u32(np, "phy-reset-duration", &bp->phy_reset_ms);
> 
> Where is the binding?

Ditto.

Cheers,
Andrea

> 
> > +	/* A sane reset duration should not be longer than 1s */
> > +	if (bp->phy_reset_ms > 1000)
> > +		bp->phy_reset_ms = 1000;
> > +
> >  	/* IP specific init */
> >  	err = init(pdev);
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index ea71612f6b36..1d298f0cf685 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -85,6 +85,8 @@ 
 #define GEM_PBUFRXCUT		0x0044 /* RX Partial Store and Forward */
 #define GEM_JML			0x0048 /* Jumbo Max Length */
 #define GEM_HS_MAC_CONFIG	0x0050 /* GEM high speed config */
+#define GEM_AMP			0x0054 /* AXI Max Pipeline */
+#define GEM_INTMOD		0x005c /* Interrupt moderation */
 #define GEM_HRB			0x0080 /* Hash Bottom */
 #define GEM_HRT			0x0084 /* Hash Top */
 #define GEM_SA1B		0x0088 /* Specific1 Bottom */
@@ -347,6 +349,21 @@ 
 #define GEM_ADDR64_OFFSET	30 /* Address bus width - 64b or 32b */
 #define GEM_ADDR64_SIZE		1
 
+/* Bitfields in AMP */
+#define GEM_AR2R_MAX_PIPE_OFFSET	0  /* Maximum number of outstanding AXI read requests */
+#define GEM_AR2R_MAX_PIPE_SIZE		8
+#define GEM_AW2W_MAX_PIPE_OFFSET	8  /* Maximum number of outstanding AXI write requests */
+#define GEM_AW2W_MAX_PIPE_SIZE		8
+#define GEM_AW2B_FILL_OFFSET		16 /* Select wether the max AW2W transactions operates between: */
+#define GEM_AW2B_FILL_AW2W		0  /*   0: the AW to W AXI channel */
+#define GEM_AW2B_FILL_AW2B		1  /*   1: AW to B channel */
+#define GEM_AW2B_FILL_SIZE              1
+
+/* Bitfields in INTMOD */
+#define GEM_RX_MODERATION_OFFSET	0  /* RX interrupt moderation */
+#define GEM_RX_MODERATION_SIZE		8
+#define GEM_TX_MODERATION_OFFSET	16 /* TX interrupt moderation */
+#define GEM_TX_MODERATION_SIZE		8
 
 /* Bitfields in PBUFRXCUT */
 #define GEM_ENCUTTHRU_OFFSET	31 /* Enable RX partial store and forward */
@@ -812,6 +829,7 @@ 
 	})
 
 #define MACB_READ_NSR(bp)	macb_readl(bp, NSR)
+#define MACB_READ_TSR(bp)	macb_readl(bp, TSR)
 
 /* struct macb_dma_desc - Hardware DMA descriptor
  * @addr: DMA address of data buffer
@@ -1228,6 +1246,7 @@  struct macb_queue {
 	dma_addr_t		tx_ring_dma;
 	struct work_struct	tx_error_task;
 	bool			txubr_pending;
+	bool			tx_pending;
 	struct napi_struct	napi_tx;
 
 	dma_addr_t		rx_ring_dma;
@@ -1293,9 +1312,15 @@  struct macb {
 
 	u32			caps;
 	unsigned int		dma_burst_length;
+	u8			aw2w_max_pipe;
+	u8			ar2r_max_pipe;
+	bool			use_aw2b_fill;
 
 	phy_interface_t		phy_interface;
 
+	struct gpio_desc	*phy_reset_gpio;
+	int			phy_reset_ms;
+
 	/* AT91RM9200 transmit queue (1 on wire + 1 queued) */
 	struct macb_tx_skb	rm9200_txq[2];
 	unsigned int		max_tx_length;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 11665be3a22c..5eb5be6c96fc 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -41,6 +41,9 @@ 
 #include <linux/inetdevice.h>
 #include "macb.h"
 
+static unsigned int txdelay = 35;
+module_param(txdelay, uint, 0644);
+
 /* This structure is only used for MACB on SiFive FU540 devices */
 struct sifive_fu540_macb_mgmt {
 	void __iomem *reg;
@@ -334,7 +337,7 @@  static int macb_mdio_wait_for_idle(struct macb *bp)
 	u32 val;
 
 	return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
-				  1, MACB_MDIO_TIMEOUT);
+				  100, MACB_MDIO_TIMEOUT);
 }
 
 static int macb_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
@@ -493,6 +496,19 @@  static int macb_mdio_write_c45(struct mii_bus *bus, int mii_id,
 	return status;
 }
 
+static int macb_mdio_reset(struct mii_bus *bus)
+{
+	struct macb *bp = bus->priv;
+
+	if (bp->phy_reset_gpio) {
+		gpiod_set_value_cansleep(bp->phy_reset_gpio, 1);
+		msleep(bp->phy_reset_ms);
+		gpiod_set_value_cansleep(bp->phy_reset_gpio, 0);
+	}
+
+	return 0;
+}
+
 static void macb_init_buffers(struct macb *bp)
 {
 	struct macb_queue *queue;
@@ -969,6 +985,7 @@  static int macb_mii_init(struct macb *bp)
 	bp->mii_bus->write = &macb_mdio_write_c22;
 	bp->mii_bus->read_c45 = &macb_mdio_read_c45;
 	bp->mii_bus->write_c45 = &macb_mdio_write_c45;
+	bp->mii_bus->reset = &macb_mdio_reset;
 	snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
 		 bp->pdev->name, bp->pdev->id);
 	bp->mii_bus->priv = bp;
@@ -1640,6 +1657,11 @@  static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
 
 		macb_init_rx_ring(queue);
 		queue_writel(queue, RBQP, queue->rx_ring_dma);
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+			macb_writel(bp, RBQPH,
+				    upper_32_bits(queue->rx_ring_dma));
+#endif
 
 		macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
 
@@ -1940,8 +1962,9 @@  static irqreturn_t macb_interrupt(int irq, void *dev_id)
 				queue_writel(queue, ISR, MACB_BIT(TCOMP) |
 							 MACB_BIT(TXUBR));
 
-			if (status & MACB_BIT(TXUBR)) {
+			if (status & MACB_BIT(TXUBR) || queue->tx_pending) {
 				queue->txubr_pending = true;
+				queue->tx_pending = 0;
 				wmb(); // ensure softirq can see update
 			}
 
@@ -2394,6 +2417,11 @@  static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	skb_tx_timestamp(skb);
 
 	spin_lock_irq(&bp->lock);
+
+	/* TSTART write might get dropped, so make the IRQ retrigger a buffer read */
+	if (macb_readl(bp, TSR) & MACB_BIT(TGO))
+		queue->tx_pending = 1;
+
 	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
 	spin_unlock_irq(&bp->lock);
 
@@ -2800,6 +2828,37 @@  static void macb_configure_dma(struct macb *bp)
 	}
 }
 
+static void gem_init_axi(struct macb *bp)
+{
+	u32 amp;
+
+	/* AXI pipeline setup - don't touch values unless specified in device
+	 * tree. Some hardware could have reset values > 1.
+	 */
+	amp = gem_readl(bp, AMP);
+
+	if (bp->use_aw2b_fill)
+		amp = GEM_BFINS(AW2B_FILL, bp->use_aw2b_fill, amp);
+	if (bp->aw2w_max_pipe)
+		amp = GEM_BFINS(AW2W_MAX_PIPE, bp->aw2w_max_pipe, amp);
+	if (bp->ar2r_max_pipe)
+		amp = GEM_BFINS(AR2R_MAX_PIPE, bp->ar2r_max_pipe, amp);
+
+	gem_writel(bp, AMP, amp);
+}
+
+static void gem_init_intmod(struct macb *bp)
+{
+	unsigned int throttle;
+	u32 intmod = 0;
+
+	/* Use sensible interrupt moderation thresholds (50us rx and tx) */
+	throttle = (1000 * 50) / 800;
+	intmod = GEM_BFINS(TX_MODERATION, throttle, intmod);
+	intmod = GEM_BFINS(RX_MODERATION, throttle, intmod);
+	gem_writel(bp, INTMOD, intmod);
+}
+
 static void macb_init_hw(struct macb *bp)
 {
 	u32 config;
@@ -2828,6 +2887,11 @@  static void macb_init_hw(struct macb *bp)
 	if (bp->caps & MACB_CAPS_JUMBO)
 		bp->rx_frm_len_mask = MACB_RX_JFRMLEN_MASK;
 
+	if (macb_is_gem(bp)) {
+		gem_init_axi(bp);
+		gem_init_intmod(bp);
+	}
+
 	macb_configure_dma(bp);
 
 	/* Enable RX partial store and forward and set watermark */
@@ -3189,6 +3253,52 @@  static void gem_get_ethtool_strings(struct net_device *dev, u32 sset, u8 *p)
 	}
 }
 
+static int gem_set_coalesce(struct net_device *dev,
+			    struct ethtool_coalesce *ec,
+			    struct kernel_ethtool_coalesce *kernel_coal,
+			    struct netlink_ext_ack *extack)
+{
+	struct macb *bp = netdev_priv(dev);
+	unsigned int tx_throttle;
+	unsigned int rx_throttle;
+	u32 intmod = 0;
+
+	/* GEM has simple IRQ throttling support. RX and TX interrupts
+	 * are separately moderated on 800ns quantums, with no support
+	 * for frame coalescing.
+	 */
+
+	/* Max is 255 * 0.8us = 204us. Zero implies no moderation. */
+	if (ec->rx_coalesce_usecs > 204 || ec->tx_coalesce_usecs > 204)
+		return -EINVAL;
+
+	tx_throttle = (1000 * ec->tx_coalesce_usecs) / 800;
+	rx_throttle = (1000 * ec->rx_coalesce_usecs) / 800;
+
+	intmod = GEM_BFINS(TX_MODERATION, tx_throttle, intmod);
+	intmod = GEM_BFINS(RX_MODERATION, rx_throttle, intmod);
+
+	gem_writel(bp, INTMOD, intmod);
+
+	return 0;
+}
+
+static int gem_get_coalesce(struct net_device *dev,
+			    struct ethtool_coalesce *ec,
+			    struct kernel_ethtool_coalesce *kernel_coal,
+			    struct netlink_ext_ack *extack)
+{
+	struct macb *bp = netdev_priv(dev);
+	u32 intmod;
+
+	intmod = gem_readl(bp, INTMOD);
+
+	ec->tx_coalesce_usecs = (GEM_BFEXT(TX_MODERATION, intmod) * 800) / 1000;
+	ec->rx_coalesce_usecs = (GEM_BFEXT(RX_MODERATION, intmod) * 800) / 1000;
+
+	return 0;
+}
+
 static struct net_device_stats *macb_get_stats(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
@@ -3772,6 +3882,8 @@  static const struct ethtool_ops macb_ethtool_ops = {
 };
 
 static const struct ethtool_ops gem_ethtool_ops = {
+	.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS |
+				     ETHTOOL_COALESCE_TX_USECS,
 	.get_regs_len		= macb_get_regs_len,
 	.get_regs		= macb_get_regs,
 	.get_wol		= macb_get_wol,
@@ -3781,6 +3893,8 @@  static const struct ethtool_ops gem_ethtool_ops = {
 	.get_ethtool_stats	= gem_get_ethtool_stats,
 	.get_strings		= gem_get_ethtool_strings,
 	.get_sset_count		= gem_get_sset_count,
+	.get_coalesce		= gem_get_coalesce,
+	.set_coalesce		= gem_set_coalesce,
 	.get_link_ksettings     = macb_get_link_ksettings,
 	.set_link_ksettings     = macb_set_link_ksettings,
 	.get_ringparam		= macb_get_ringparam,
@@ -5100,6 +5214,11 @@  static int macb_probe(struct platform_device *pdev)
 			}
 		}
 	}
+
+	device_property_read_u8(&pdev->dev, "cdns,aw2w-max-pipe", &bp->aw2w_max_pipe);
+	device_property_read_u8(&pdev->dev, "cdns,ar2r-max-pipe", &bp->ar2r_max_pipe);
+	bp->use_aw2b_fill = device_property_read_bool(&pdev->dev, "cdns,use-aw2b-fill");
+
 	spin_lock_init(&bp->lock);
 
 	/* setup capabilities */
@@ -5155,6 +5274,21 @@  static int macb_probe(struct platform_device *pdev)
 	else
 		bp->phy_interface = interface;
 
+	/* optional PHY reset-related properties */
+	bp->phy_reset_gpio = devm_gpiod_get_optional(&pdev->dev, "phy-reset",
+						     GPIOD_OUT_LOW);
+	if (IS_ERR(bp->phy_reset_gpio)) {
+		dev_err(&pdev->dev, "Failed to obtain phy-reset gpio\n");
+		err = PTR_ERR(bp->phy_reset_gpio);
+		goto err_out_free_netdev;
+	}
+
+	bp->phy_reset_ms = 10;
+	of_property_read_u32(np, "phy-reset-duration", &bp->phy_reset_ms);
+	/* A sane reset duration should not be longer than 1s */
+	if (bp->phy_reset_ms > 1000)
+		bp->phy_reset_ms = 1000;
+
 	/* IP specific init */
 	err = init(pdev);
 	if (err)
@@ -5229,6 +5363,19 @@  static void macb_remove(struct platform_device *pdev)
 	}
 }
 
+static void macb_shutdown(struct platform_device *pdev)
+{
+	struct net_device *dev;
+
+	dev = platform_get_drvdata(pdev);
+
+	rtnl_lock();
+	netif_device_detach(dev);
+	if (netif_running(dev))
+		dev_close(dev);
+	rtnl_unlock();
+}
+
 static int __maybe_unused macb_suspend(struct device *dev)
 {
 	struct net_device *netdev = dev_get_drvdata(dev);
@@ -5482,6 +5629,7 @@  static const struct dev_pm_ops macb_pm_ops = {
 static struct platform_driver macb_driver = {
 	.probe		= macb_probe,
 	.remove_new	= macb_remove,
+	.shutdown	= macb_shutdown,
 	.driver		= {
 		.name		= "macb",
 		.of_match_table	= of_match_ptr(macb_dt_ids),