| Message ID | 20231222054451.2683242-4-leong.ching.swee@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: stmmac: Enable Per DMA Channel interrupt | expand |
> -----Original Message----- > From: Serge Semin <fancer.lancer@gmail.com> > Sent: Saturday, December 23, 2023 5:49 AM > To: Swee, Leong Ching <leong.ching.swee@intel.com> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>; Alexandre Torgue > <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>; > David S . Miller <davem@davemloft.net>; Eric Dumazet > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni > <pabeni@redhat.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof > Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > <conor+dt@kernel.org>; Giuseppe Cavallaro <peppe.cavallaro@st.com>; > linux-stm32@st-md-mailman.stormreply.com; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > netdev@vger.kernel.org; devicetree@vger.kernel.org; Teoh Ji Sheng > <ji.sheng.teoh@intel.com> > Subject: Re: [PATCH net-next v1 3/4] net: stmmac: Add support for TX/RX > channel interrupt > > On Fri, Dec 22, 2023 at 01:44:50PM +0800, Leong Ching Swee wrote: > > From: Swee Leong Ching <leong.ching.swee@intel.com> > > > > Enable TX/RX channel interrupt registration for MAC that interrupts > > CPU through shared peripheral interrupt (SPI). > > > > Per channel interrupts and interrupt-names are registered through, > > Eg: 4 tx and 4 rx channels: > > interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; > > <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>; > > <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>; > > <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>; > > <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>; interrupt-names = > > "dma_tx0", > > "dma_tx1", > > "dma_tx2", > > "dma_tx3", > > "dma_rx0", > > "dma_rx1", > > "dma_rx2", > > "dma_rx3"; > > > > Signed-off-by: Teoh Ji Sheng <ji.sheng.teoh@intel.com> > > Signed-off-by: Swee Leong Ching <leong.ching.swee@intel.com> > > --- > > .../ethernet/stmicro/stmmac/stmmac_platform.c | 24 > > +++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > index 70eadc83ca68..f857907f13a0 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > @@ -710,6 +710,8 @@ > EXPORT_SYMBOL_GPL(devm_stmmac_probe_config_dt); > > int stmmac_get_platform_resources(struct platform_device *pdev, > > struct stmmac_resources *stmmac_res) { > > > + char irq_name[8]; > > By DW XGMAC v2.x IP-core design there can be up to 16 Tx channels and > 12 Rx channels. Thus it's better to set irq_name size being at least > (strlen("dma_tx16") + 1) == 9 beforehand since you are adding this code > anyway and for some reason didn't consider to pick the Jisheng' > patch up which fixed the MTL_MAX_TX_QUEUES/MTL_MAX_RX_QUEUES > macros. > I only have 8 channels tx/rx dma irq setup, so I could not test 16 channels Patch. Will update to 9 in v2. > > + int i; > > Please add an empty line between the variables declaration and the next > statement. > Thanks. Will update this in v2. > > memset(stmmac_res, 0, sizeof(*stmmac_res)); > > > > /* Get IRQ information early to have an ability to ask for deferred > > @@ -719,6 +721,28 @@ int stmmac_get_platform_resources(struct > platform_device *pdev, > > if (stmmac_res->irq < 0) > > return stmmac_res->irq; > > > > > + /* For RX Channel */ > > + for (i = 0; i < MTL_MAX_RX_QUEUES; i++) { > > + snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i); > > + stmmac_res->rx_irq[i] = > platform_get_irq_byname_optional(pdev, irq_name); > > + if (stmmac_res->rx_irq[i] < 0) { > > + if (stmmac_res->rx_irq[i] == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + break; > > + } > > + } > > What about: > > + /* Get optional Tx/Rx DMA per-channel IRQs, which otherwise > + * are supposed to be delivered via the common MAC IRQ line > + */ > + for (i = 0; i < MTL_MAX_RX_QUEUES; i++) { > + snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i); > + irq = platform_get_irq_byname_optional(pdev, irq_name); > + if (irq == -EPROBE_DEFER) > + return irq; > + else if (irq < 0) > + break; > + > + stmmac_res->rx_irq[i] = irq; > + } > > It's cleaner a bit with less indentations and doesn't pollute rx_irq[]/tx_irq[] > arrays with the error numbers. > Sure, will update this in v2. > > + > > + /* For TX Channel */ > > + for (i = 0; i < MTL_MAX_TX_QUEUES; i++) { > > + snprintf(irq_name, sizeof(irq_name), "dma_tx%i", i); > > + stmmac_res->tx_irq[i] = > platform_get_irq_byname_optional(pdev, irq_name); > > + if (stmmac_res->tx_irq[i] < 0) { > > + if (stmmac_res->rx_irq[i] == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + break; > > + } > > + } > > + > > Please move the Tx/Rx IRQs getting loops to the bottom of the > stmmac_get_platform_resources() method. Thus the order of the IRQs > getting would be the same as the order of the IRQs requests implemented in > the stmmac_request_irq_multi_msi() and > stmmac_request_irq_single() methods. > > -Serge(y) > Will move those methods to bottom in v2. > > /* On some platforms e.g. SPEAr the wake up irq differs from the > mac irq > > * The external wake up irq can be passed through the platform code > > * named as "eth_wake_irq" > > -- > > 2.34.1 > > > >
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index 70eadc83ca68..f857907f13a0 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -710,6 +710,8 @@ EXPORT_SYMBOL_GPL(devm_stmmac_probe_config_dt); int stmmac_get_platform_resources(struct platform_device *pdev, struct stmmac_resources *stmmac_res) { + char irq_name[8]; + int i; memset(stmmac_res, 0, sizeof(*stmmac_res)); /* Get IRQ information early to have an ability to ask for deferred @@ -719,6 +721,28 @@ int stmmac_get_platform_resources(struct platform_device *pdev, if (stmmac_res->irq < 0) return stmmac_res->irq; + /* For RX Channel */ + for (i = 0; i < MTL_MAX_RX_QUEUES; i++) { + snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i); + stmmac_res->rx_irq[i] = platform_get_irq_byname_optional(pdev, irq_name); + if (stmmac_res->rx_irq[i] < 0) { + if (stmmac_res->rx_irq[i] == -EPROBE_DEFER) + return -EPROBE_DEFER; + break; + } + } + + /* For TX Channel */ + for (i = 0; i < MTL_MAX_TX_QUEUES; i++) { + snprintf(irq_name, sizeof(irq_name), "dma_tx%i", i); + stmmac_res->tx_irq[i] = platform_get_irq_byname_optional(pdev, irq_name); + if (stmmac_res->tx_irq[i] < 0) { + if (stmmac_res->rx_irq[i] == -EPROBE_DEFER) + return -EPROBE_DEFER; + break; + } + } + /* On some platforms e.g. SPEAr the wake up irq differs from the mac irq * The external wake up irq can be passed through the platform code * named as "eth_wake_irq"