From: Thierry Reding <thierry.reding@gmail•com>
To: Joao Pinto <Joao.Pinto@synopsys•com>
Cc: davem@davemloft•net, peppe.cavallaro@st•com,
alexandre.torgue@st•com, niklas.cassel@axis•com,
netdev@vger•kernel.org
Subject: Re: [v5,net-next,2/9] net: stmmac: configure mtl rx and tx algorithms
Date: Tue, 21 Mar 2017 13:24:26 +0100 [thread overview]
Message-ID: <20170321122426.GE30407@ulmo.ba.sec> (raw)
In-Reply-To: <1020202a-5b96-0c11-a5de-fbd93ff02323@synopsys.com>
[-- Attachment #1: Type: text/plain, Size: 8945 bytes --]
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 <jpinto@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://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c#n156
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.
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2017-03-21 12:33 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-10 18:24 [PATCH v5 net-next 0/9] prepare mac operations for multiple queues Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 1/9] net: stmmac: multiple queues dt configuration Joao Pinto
[not found] ` <c315778b650a846db3c04816fd1d1041bb1e4043.1489169213.git.jpinto-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2017-03-21 11:32 ` [v5,net-next,1/9] " Thierry Reding
[not found] ` <20170321113245.GA10526-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org>
2017-03-21 11:39 ` Joao Pinto
[not found] ` <340aba35-de12-14a8-749c-5507edab449e-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
2017-03-21 12:10 ` Thierry Reding
2017-03-10 18:24 ` [PATCH v5 net-next 2/9] net: stmmac: configure mtl rx and tx algorithms Joao Pinto
2017-03-21 11:58 ` [v5,net-next,2/9] " Thierry Reding
2017-03-21 12:02 ` Joao Pinto
2017-03-21 12:24 ` Thierry Reding [this message]
2017-03-21 13:58 ` Joao Pinto
2017-03-21 14:08 ` Thierry Reding
2017-03-21 14:10 ` Joao Pinto
2017-03-21 14:23 ` Corentin Labbe
2017-03-21 14:25 ` Joao Pinto
2017-03-21 14:33 ` Thierry Reding
2017-03-21 14:35 ` Joao Pinto
2017-03-21 14:18 ` Joao Pinto
2017-03-21 14:35 ` Thierry Reding
2017-03-10 18:24 ` [PATCH v5 net-next 3/9] net: stmmac: configure tx queue weight Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 4/9] net: stmmac: mtl rx queue enabled as dcb or avb Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 5/9] net: stmmac: mapping mtl rx to dma channel Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 6/9] net: stmmac: flow_ctrl functions adapted to mtl Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 7/9] net: stmmac: prepare irq_status for mtl Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 8/9] net: stmmac: mac debug prepared for multiple queues Joao Pinto
2017-03-10 18:24 ` [PATCH v5 net-next 9/9] net: stmmac: configuration of CBS in case of a TX AVB queue Joao Pinto
2017-03-13 6:42 ` [PATCH v5 net-next 0/9] prepare mac operations for multiple queues David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170321122426.GE30407@ulmo.ba.sec \
--to=thierry.reding@gmail$(echo .)com \
--cc=Joao.Pinto@synopsys$(echo .)com \
--cc=alexandre.torgue@st$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=niklas.cassel@axis$(echo .)com \
--cc=peppe.cavallaro@st$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox