| Message ID | 775000dfb3a35bc691010072942253cb022750e1.1724159867.git.andrea.porta@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for RaspberryPi RP1 PCI device using a DT overlay | expand |
> +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
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
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
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?
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 >
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 --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),
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(-)