Às 12:24 PM de 3/21/2017, Thierry Reding escreveu: > On Tue, Mar 21, 2017 at 12:02:03PM +0000, Joao Pinto wrote: >> Às 11:58 AM de 3/21/2017, Thierry Reding escreveu: >>> On Fri, Mar 10, 2017 at 06:24:52PM +0000, Joao Pinto wrote: >>>> This patch adds the RX and TX scheduling algorithms programming. >>>> It introduces the multiple queues configuration function >>>> (stmmac_mtl_configuration) in stmmac_main. >>>> >>>> Signed-off-by: Joao Pinto <jpi...@synopsys.com> >>>> --- >>>> Changes v4->v5: >>>> - patch title update (stmicro replaced by stmmac) >>>> Changes v3->v4: >>>> - Just to keep up with patch-set version >>>> Changes v2->v3: >>>> - Switch statements with a tab >>>> Changes v1->v2: >>>> - Just to keep up with patch-set version >>>> >>>> drivers/net/ethernet/stmicro/stmmac/common.h | 4 ++ >>>> drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 10 +++++ >>>> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 48 >>>> +++++++++++++++++++++++ >>>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++++++++-- >>>> 4 files changed, 90 insertions(+), 3 deletions(-) >>> >>> This patch breaks backwards-compatibility with DTBs that don't have an >>> of the multiple queue properties. >>> >>> See below... >>> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h >>>> b/drivers/net/ethernet/stmicro/stmmac/common.h >>>> index 04d9245..5a0a781 100644 >>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h >>>> @@ -455,6 +455,10 @@ struct stmmac_ops { >>>> int (*rx_ipc)(struct mac_device_info *hw); >>>> /* Enable RX Queues */ >>>> void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue); >>>> + /* Program RX Algorithms */ >>>> + void (*prog_mtl_rx_algorithms)(struct mac_device_info *hw, u32 rx_alg); >>>> + /* Program TX Algorithms */ >>>> + void (*prog_mtl_tx_algorithms)(struct mac_device_info *hw, u32 tx_alg); >>>> /* Dump MAC registers */ >>>> void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space); >>>> /* Handle extra events on specific interrupts hw dependent */ >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h >>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h >>>> index db45134..748ab6f 100644 >>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h >>>> @@ -161,6 +161,16 @@ enum power_event { >>>> #define GMAC_HI_REG_AE BIT(31) >>>> >>>> /* MTL registers */ >>>> +#define MTL_OPERATION_MODE 0x00000c00 >>>> +#define MTL_OPERATION_SCHALG_MASK GENMASK(6, 5) >>>> +#define MTL_OPERATION_SCHALG_WRR (0x0 << 5) >>>> +#define MTL_OPERATION_SCHALG_WFQ (0x1 << 5) >>>> +#define MTL_OPERATION_SCHALG_DWRR (0x2 << 5) >>>> +#define MTL_OPERATION_SCHALG_SP (0x3 << 5) >>>> +#define MTL_OPERATION_RAA BIT(2) >>>> +#define MTL_OPERATION_RAA_SP (0x0 << 2) >>>> +#define MTL_OPERATION_RAA_WSP (0x1 << 2) >>>> + >>>> #define MTL_INT_STATUS 0x00000c20 >>>> #define MTL_INT_Q0 BIT(0) >>>> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >>>> index 1e79e65..f966755 100644 >>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >>>> @@ -70,6 +70,52 @@ static void dwmac4_rx_queue_enable(struct >>>> mac_device_info *hw, u32 queue) >>>> writel(value, ioaddr + GMAC_RXQ_CTRL0); >>>> } >>>> >>>> +static void dwmac4_prog_mtl_rx_algorithms(struct mac_device_info *hw, >>>> + u32 rx_alg) >>>> +{ >>>> + void __iomem *ioaddr = hw->pcsr; >>>> + u32 value = readl(ioaddr + MTL_OPERATION_MODE); >>>> + >>>> + value &= ~MTL_OPERATION_RAA; >>>> + switch (rx_alg) { >>>> + case MTL_RX_ALGORITHM_SP: >>>> + value |= MTL_OPERATION_RAA_SP; >>>> + break; >>>> + case MTL_RX_ALGORITHM_WSP: >>>> + value |= MTL_OPERATION_RAA_WSP; >>>> + break; >>>> + default: >>>> + break; >>>> + } >>>> + >>>> + writel(value, ioaddr + MTL_OPERATION_MODE); >>>> +} >>>> + >>>> +static void dwmac4_prog_mtl_tx_algorithms(struct mac_device_info *hw, >>>> + u32 tx_alg) >>>> +{ >>>> + void __iomem *ioaddr = hw->pcsr; >>>> + u32 value = readl(ioaddr + MTL_OPERATION_MODE); >>>> + >>>> + value &= ~MTL_OPERATION_SCHALG_MASK; >>>> + switch (tx_alg) { >>>> + case MTL_TX_ALGORITHM_WRR: >>>> + value |= MTL_OPERATION_SCHALG_WRR; >>>> + break; >>>> + case MTL_TX_ALGORITHM_WFQ: >>>> + value |= MTL_OPERATION_SCHALG_WFQ; >>>> + break; >>>> + case MTL_TX_ALGORITHM_DWRR: >>>> + value |= MTL_OPERATION_SCHALG_DWRR; >>>> + break; >>>> + case MTL_TX_ALGORITHM_SP: >>>> + value |= MTL_OPERATION_SCHALG_SP; >>>> + break; >>>> + default: >>>> + break; >>>> + } >>>> +} >>>> + >>>> static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space) >>>> { >>>> void __iomem *ioaddr = hw->pcsr; >>>> @@ -457,6 +503,8 @@ static const struct stmmac_ops dwmac4_ops = { >>>> .core_init = dwmac4_core_init, >>>> .rx_ipc = dwmac4_rx_ipc_enable, >>>> .rx_queue_enable = dwmac4_rx_queue_enable, >>>> + .prog_mtl_rx_algorithms = dwmac4_prog_mtl_rx_algorithms, >>>> + .prog_mtl_tx_algorithms = dwmac4_prog_mtl_tx_algorithms, >>>> .dump_regs = dwmac4_dump_regs, >>>> .host_irq_status = dwmac4_irq_status, >>>> .flow_ctrl = dwmac4_flow_ctrl, >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>> index 4498a38..af57f8d 100644 >>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>> @@ -1645,6 +1645,31 @@ static void stmmac_init_tx_coalesce(struct >>>> stmmac_priv *priv) >>>> } >>>> >>>> /** >>>> + * stmmac_mtl_configuration - Configure MTL >>>> + * @priv: driver private structure >>>> + * Description: It is used for configurring MTL >>>> + */ >>>> +static void stmmac_mtl_configuration(struct stmmac_priv *priv) >>>> +{ >>>> + u32 rx_queues_count = priv->plat->rx_queues_to_use; >>>> + u32 tx_queues_count = priv->plat->tx_queues_to_use; >>>> + >>>> + /* Configure MTL RX algorithms */ >>>> + if (rx_queues_count > 1 && priv->hw->mac->prog_mtl_rx_algorithms) >>>> + priv->hw->mac->prog_mtl_rx_algorithms(priv->hw, >>>> + priv->plat->rx_sched_algorithm); >>>> + >>>> + /* Configure MTL TX algorithms */ >>>> + if (tx_queues_count > 1 && priv->hw->mac->prog_mtl_tx_algorithms) >>>> + priv->hw->mac->prog_mtl_tx_algorithms(priv->hw, >>>> + priv->plat->tx_sched_algorithm); >>>> + >>>> + /* Enable MAC RX Queues */ >>>> + if (rx_queues_count > 1 && priv->hw->mac->rx_queue_enable) >>>> + stmmac_mac_enable_rx_queues(priv); >>> >>> This is almost equivalent to the code removed from stmmac_hw_setup() >>> which happens to be the key for this driver to work for me. However, the >>> code above adds an additional check for rx_queues_count > 1 which is >>> going to be false for any existing DTB, because it is derived from the >>> values retrieved from new device tree properties. >>> >>> So I think for backwards compatibility we'd need something like this: >>> >>> if ((rx_queue_count == 0 || rx_queue_count > 1) && >>> priv->hw->mac->rx_queue_enable) >>> >>> But then I'm beginning to think maybe we don't need a check here at all >>> because it would only prevent RX queue setup for rx_queue_count == 1 and >>> I think it would still be legitimate to set it up even then. >>> >>> stmmac_mac_enable_rx_queues() already checks for rx_count == 1, but that >>> is derived from the number of RX queues derived from the feature >>> registers and therefore refers to the number of queues that the hardware >>> supports as opposed to the number of queues configured in device tree. >>> >>> I can follow up with a patch to restore backwards-compatibility. >> >> Forhw configured as single queue you don't need to enable the rx queue, since >> they are enable by default. if you check in stmmac_platform.c I assure >> backward >> compatibility by setting the number of rx and tx queues = 1 if nothing is >> declared. Please check here: >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_davem_net-2Dnext.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n156&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=tvYpf8cZr-Y6Zl-d7NlT2FEW7Gv5Bzs86GMIhZs4OSI&s=jA0P7-MXCVOtu262fF75EhUq5dfcDutzsYg8CAE6XQc&e= >> > > Ah yes, I just ran across that trying to debug why a subsequent patch > broke things again. I think the rx_queue_count > 1 condition is still > wrong in the above because it will still fail for the backwards- > compatibility case.
Enabling RX Queue is needed only if the hw is configured as having more than 1 RX queue. If your hardware is configured as being multiple-queue, rx_queue_to_use should be > 1. True single queue hardware will work fine. > > I still think any check other than priv->hw->mac->rx_queue_enable should > be dropped when calling stmmac_mac_enable_rx_queues(). A later patch is > going to move the check into that function anyway (via the for loop). I > need to step through the series some more since there are a couple of > other things I noticed that make the driver fail for Tegra186 now. Let > me report back once I'm through all of it. > > Thierry >