Date:   Thu, 9 Mar 2017 10:37:42 +0100
From:   Niklas Cassel <niklas.cassel@...s.com>
To:     Joao Pinto <Joao.Pinto@...opsys.com>, <davem@...emloft.net>
CC:     <peppe.cavallaro@...com>, <alexandre.torgue@...com>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next 2/8] net: stmicro: configure mtl rx and tx
 algorithms

On 03/08/2017 05:48 PM, Joao Pinto wrote:
> Às 4:45 PM de 3/8/2017, Niklas Cassel escreveu:
>> On 03/08/2017 01:22 PM, 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@...opsys.com>
>>> ---
>>> 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(-)
>>>
>>> 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..7503b8e 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);
>> Hello Joao
>>
>> Since you are now enabling RX queues here,
>> perhaps we should move the enabling of TX queue(s) here as well?
> Hi Niklas,
> TX enable operation is different, it is part of the DMA Operation Mode
> configuration. In a later patch I will be updating the DMA Operation Mode
> configuration and this op will be placed here.
>
> As indicated by David Miller, this patch-set is focused in MAC ops only. DMA
> focused patch-set will follow after this one gets accepted.

Not sure that I agree 100% since:

"23.2 Initializing MTL Registers"

1. Program the Tx Scheduling algorithm. (Which you now do in stmmac_mtl_configuration.)

[snip]

3. Program the following fields to initialize the mode of operation in the MTL_TxQ0_Operation_Mode
a. Transmit Store And Forward (TSF) or Transmit Threshold Control (TTC) in case of threshold mode
b. Transmit Queue Enable (TXQEN) to value 2‘b10 to enable Transmit Queue0
c. Transmit Queue Size (TQS)


Note that "Initializing DMA" is a separate chapter.

However, since you are planning on moving the MTL_TxQ0_Operation_Mode initialization
to stmmac_mtl_configuration in a later patch, I don't think that you have to rework this
patch just because of this simple remark.

Nice to see some work done on multiqueues for stmmac :)


>
> Thanks!
>
>>
>>> +}
>>> +
>>> +/**
>>>   * stmmac_hw_setup - setup mac in a usable state.
>>>   *  @dev : pointer to the device structure.
>>>   *  Description:
>>> @@ -1688,9 +1713,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
>>>  	/* Initialize the MAC Core */
>>>  	priv->hw->mac->core_init(priv->hw, dev->mtu);
>>>  
>>> -	/* Initialize MAC RX Queues */
>>> -	if (priv->hw->mac->rx_queue_enable)
>>> -		stmmac_mac_enable_rx_queues(priv);
>>> +	/* Initialize MTL*/
>>> +	if (priv->synopsys_id >= DWMAC_CORE_4_00)
>>> +		stmmac_mtl_configuration(priv);
>>>  
>>>  	ret = priv->hw->mac->rx_ipc(priv->hw);
>>>  	if (!ret) {

Powered by blists - more mailing lists