diff mbox

net: stmmac: enable tx queue 0 for gmac4 IPs synthesized with multiple TX queues

Message ID 1479998194-7113-1-git-send-email-niklass@axis.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Niklas Cassel Nov. 24, 2016, 2:36 p.m. UTC
From: Niklas Cassel <niklas.cassel@axis.com>

The dwmac4 IP can synthesized with 1-8 number of tx queues.
On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
by default. For these IPs, the bitfield TXQEN is R/W.

Always enable tx queue 0. The write will have no effect on IPs synthesized
with DWC_EQOS_NUM_TXQ == 1.

The driver does still not utilize more than one tx queue in the IP.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Alexandre TORGUE Nov. 24, 2016, 6:11 p.m. UTC | #1
Hi Niklas,

On 11/24/2016 03:36 PM, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@axis.com>
>
> The dwmac4 IP can synthesized with 1-8 number of tx queues.
> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
> by default. For these IPs, the bitfield TXQEN is R/W.
>
> Always enable tx queue 0. The write will have no effect on IPs synthesized
> with DWC_EQOS_NUM_TXQ == 1.
>
> The driver does still not utilize more than one tx queue in the IP.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> index 6f4f5ce25114..3e8d4fefa5e0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> @@ -155,8 +155,11 @@ enum power_event {
>  #define MTL_CHAN_RX_DEBUG(x)		(MTL_CHANX_BASE_ADDR(x) + 0x38)
>
>  #define MTL_OP_MODE_RSF			BIT(5)
> +#define MTL_OP_MODE_TXQEN		BIT(3)
>  #define MTL_OP_MODE_TSF			BIT(1)
>
> +#define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
> +
>  #define MTL_OP_MODE_TTC_MASK		0x70
>  #define MTL_OP_MODE_TTC_SHIFT		4
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 116151cd6a95..577316de6ba8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>  		else
>  			mtl_tx_op |= MTL_OP_MODE_TTC_512;
>  	}
> -
> +	/* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
> +	 * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
> +	 * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
> +	 * with reset values: TXQEN off, TQS 256 bytes.
> +	 *
> +	 * Write the bits in both cases, since it will have no effect when RO.
> +	 * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
> +	 * be RO, however, writing the whole TQS field will result in a value
> +	 * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
> +	 */
> +	mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;

Your patch sounds good. Just one question:

In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 
can take several values "disabled / enabled / Enabled in AV mode":

Transmit Queue Enable
This field is used to enable/disable the transmit queue 1. 00 R/W
■ 2'b00 - Not enabled
■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
Bridging is not selected while configuring the core)
■ 2'b10 - Enabled
■ 2'b11 - Reserved

Do you plan to manage av mode in a future patch ?

Regards
Alex

>  	writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>
>  	mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>
Niklas Cassel Nov. 25, 2016, 12:10 p.m. UTC | #2
On 11/24/2016 07:11 PM, Alexandre Torgue wrote:
> Hi Niklas,

Hello Alexandre

>
> On 11/24/2016 03:36 PM, Niklas Cassel wrote:
>> From: Niklas Cassel <niklas.cassel@axis.com>
>>
>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>> by default. For these IPs, the bitfield TXQEN is R/W.
>>
>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>> with DWC_EQOS_NUM_TXQ == 1.
>>
>> The driver does still not utilize more than one tx queue in the IP.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> index 6f4f5ce25114..3e8d4fefa5e0 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>> @@ -155,8 +155,11 @@ enum power_event {
>>  #define MTL_CHAN_RX_DEBUG(x)        (MTL_CHANX_BASE_ADDR(x) + 0x38)
>>
>>  #define MTL_OP_MODE_RSF            BIT(5)
>> +#define MTL_OP_MODE_TXQEN        BIT(3)
>>  #define MTL_OP_MODE_TSF            BIT(1)
>>
>> +#define MTL_OP_MODE_TQS_MASK        GENMASK(24, 16)
>> +
>>  #define MTL_OP_MODE_TTC_MASK        0x70
>>  #define MTL_OP_MODE_TTC_SHIFT        4
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> index 116151cd6a95..577316de6ba8 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>          else
>>              mtl_tx_op |= MTL_OP_MODE_TTC_512;
>>      }
>> -
>> +    /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
>> +     * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
>> +     * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
>> +     * with reset values: TXQEN off, TQS 256 bytes.
>> +     *
>> +     * Write the bits in both cases, since it will have no effect when RO.
>> +     * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
>> +     * be RO, however, writing the whole TQS field will result in a value
>> +     * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
>> +     */
>> +    mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
>
> Your patch sounds good. Just one question:
>
> In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 can take several values "disabled / enabled / Enabled in AV mode":
>
> Transmit Queue Enable
> This field is used to enable/disable the transmit queue 1. 00 R/W
> ■ 2'b00 - Not enabled
> ■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
> Bridging is not selected while configuring the core)
> ■ 2'b10 - Enabled
> ■ 2'b11 - Reserved
>
> Do you plan to manage av mode in a future patch ?

We are not planning on using the AV mode.
We will probably not use TXQ1 at all.

I noticed that the MAC_HW_Feature2 Register actually has a TXQCNT field.
It is currently saved in priv->dma_cap.number_tx_channel.
If you prefer, I could do a patch V2 where we only set the bits if
priv->dma_cap.number_tx_channel > 1

However, I don't think the current patch is too bad either, since the bits
are RO when number_tx_channel == 1.


>
> Regards
> Alex
>
>>      writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>>
>>      mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>>
Niklas Cassel Nov. 25, 2016, 12:14 p.m. UTC | #3
On 11/25/2016 01:10 PM, Niklas Cassel wrote:
> On 11/24/2016 07:11 PM, Alexandre Torgue wrote:
>> Hi Niklas,
> Hello Alexandre
>
>> On 11/24/2016 03:36 PM, Niklas Cassel wrote:
>>> From: Niklas Cassel <niklas.cassel@axis.com>
>>>
>>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>>> by default. For these IPs, the bitfield TXQEN is R/W.
>>>
>>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>>> with DWC_EQOS_NUM_TXQ == 1.
>>>
>>> The driver does still not utilize more than one tx queue in the IP.
>>>
>>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>>> ---
>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>> index 6f4f5ce25114..3e8d4fefa5e0 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>> @@ -155,8 +155,11 @@ enum power_event {
>>>  #define MTL_CHAN_RX_DEBUG(x)        (MTL_CHANX_BASE_ADDR(x) + 0x38)
>>>
>>>  #define MTL_OP_MODE_RSF            BIT(5)
>>> +#define MTL_OP_MODE_TXQEN        BIT(3)
>>>  #define MTL_OP_MODE_TSF            BIT(1)
>>>
>>> +#define MTL_OP_MODE_TQS_MASK        GENMASK(24, 16)
>>> +
>>>  #define MTL_OP_MODE_TTC_MASK        0x70
>>>  #define MTL_OP_MODE_TTC_SHIFT        4
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> index 116151cd6a95..577316de6ba8 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>>          else
>>>              mtl_tx_op |= MTL_OP_MODE_TTC_512;
>>>      }
>>> -
>>> +    /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
>>> +     * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
>>> +     * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
>>> +     * with reset values: TXQEN off, TQS 256 bytes.
>>> +     *
>>> +     * Write the bits in both cases, since it will have no effect when RO.
>>> +     * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
>>> +     * be RO, however, writing the whole TQS field will result in a value
>>> +     * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
>>> +     */
>>> +    mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
>> Your patch sounds good. Just one question:
>>
>> In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 can take several values "disabled / enabled / Enabled in AV mode":
>>
>> Transmit Queue Enable
>> This field is used to enable/disable the transmit queue 1. 00 R/W
>> ■ 2'b00 - Not enabled
>> ■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
>> Bridging is not selected while configuring the core)
>> ■ 2'b10 - Enabled
>> ■ 2'b11 - Reserved
>>
>> Do you plan to manage av mode in a future patch ?
> We are not planning on using the AV mode.
> We will probably not use TXQ1 at all.
>
> I noticed that the MAC_HW_Feature2 Register actually has a TXQCNT field.
> It is currently saved in priv->dma_cap.number_tx_channel.
> If you prefer, I could do a patch V2 where we only set the bits if
> priv->dma_cap.number_tx_channel > 1

Oh, sorry, that was number of tx _channels_,
not number of tx _queues_.

However, we could add a number_tx_queue to struct dma_features,
if you would prefer that.

>
> However, I don't think the current patch is too bad either, since the bits
> are RO when number_tx_channel == 1.
>
>
>> Regards
>> Alex
>>
>>>      writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>>>
>>>      mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>>>
Alexandre TORGUE Nov. 25, 2016, 3:40 p.m. UTC | #4
Hi Niklas

On 11/25/2016 01:14 PM, Niklas Cassel wrote:
> On 11/25/2016 01:10 PM, Niklas Cassel wrote:
>> On 11/24/2016 07:11 PM, Alexandre Torgue wrote:
>>> Hi Niklas,
>> Hello Alexandre
>>
>>> On 11/24/2016 03:36 PM, Niklas Cassel wrote:
>>>> From: Niklas Cassel <niklas.cassel@axis.com>
>>>>
>>>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>>>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>>>> by default. For these IPs, the bitfield TXQEN is R/W.
>>>>
>>>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>>>> with DWC_EQOS_NUM_TXQ == 1.
>>>>
>>>> The driver does still not utilize more than one tx queue in the IP.
>>>>
>>>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>>>> ---
>>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>> index 6f4f5ce25114..3e8d4fefa5e0 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>> @@ -155,8 +155,11 @@ enum power_event {
>>>>  #define MTL_CHAN_RX_DEBUG(x)        (MTL_CHANX_BASE_ADDR(x) + 0x38)
>>>>
>>>>  #define MTL_OP_MODE_RSF            BIT(5)
>>>> +#define MTL_OP_MODE_TXQEN        BIT(3)
>>>>  #define MTL_OP_MODE_TSF            BIT(1)
>>>>
>>>> +#define MTL_OP_MODE_TQS_MASK        GENMASK(24, 16)
>>>> +
>>>>  #define MTL_OP_MODE_TTC_MASK        0x70
>>>>  #define MTL_OP_MODE_TTC_SHIFT        4
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>>> index 116151cd6a95..577316de6ba8 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>>> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>>>          else
>>>>              mtl_tx_op |= MTL_OP_MODE_TTC_512;
>>>>      }
>>>> -
>>>> +    /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
>>>> +     * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
>>>> +     * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
>>>> +     * with reset values: TXQEN off, TQS 256 bytes.
>>>> +     *
>>>> +     * Write the bits in both cases, since it will have no effect when RO.
>>>> +     * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
>>>> +     * be RO, however, writing the whole TQS field will result in a value
>>>> +     * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
>>>> +     */
>>>> +    mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
>>> Your patch sounds good. Just one question:
>>>
>>> In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 can take several values "disabled / enabled / Enabled in AV mode":
>>>
>>> Transmit Queue Enable
>>> This field is used to enable/disable the transmit queue 1. 00 R/W
>>> ■ 2'b00 - Not enabled
>>> ■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
>>> Bridging is not selected while configuring the core)
>>> ■ 2'b10 - Enabled
>>> ■ 2'b11 - Reserved
>>>
>>> Do you plan to manage av mode in a future patch ?
>> We are not planning on using the AV mode.
>> We will probably not use TXQ1 at all.
>>
>> I noticed that the MAC_HW_Feature2 Register actually has a TXQCNT field.
>> It is currently saved in priv->dma_cap.number_tx_channel.
>> If you prefer, I could do a patch V2 where we only set the bits if
>> priv->dma_cap.number_tx_channel > 1
>
> Oh, sorry, that was number of tx _channels_,
> not number of tx _queues_.
>
> However, we could add a number_tx_queue to struct dma_features,
> if you would prefer that.

I agree your patch is good. It will work even if we use several tx 
channels. We will see in the future for AV mode.

regards
alex

>
>>
>> However, I don't think the current patch is too bad either, since the bits
>> are RO when number_tx_channel == 1.
>>
>>
>>> Regards
>>> Alex
>>>
>>>>      writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>>>>
>>>>      mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>>>>
>
Niklas Cassel Nov. 25, 2016, 4:49 p.m. UTC | #5
On 11/25/2016 04:40 PM, Alexandre Torgue wrote:
> Hi Niklas
>
> On 11/25/2016 01:14 PM, Niklas Cassel wrote:
>> On 11/25/2016 01:10 PM, Niklas Cassel wrote:
>>> On 11/24/2016 07:11 PM, Alexandre Torgue wrote:
>>>> Hi Niklas,
>>> Hello Alexandre
>>>
>>>> On 11/24/2016 03:36 PM, Niklas Cassel wrote:
>>>>> From: Niklas Cassel <niklas.cassel@axis.com>
>>>>>
>>>>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>>>>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>>>>> by default. For these IPs, the bitfield TXQEN is R/W.
>>>>>
>>>>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>>>>> with DWC_EQOS_NUM_TXQ == 1.
>>>>>
>>>>> The driver does still not utilize more than one tx queue in the IP.
>>>>>
>>>>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>>>>> ---
>>>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4.h     |  3 +++
>>>>>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++++++-
>>>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>> index 6f4f5ce25114..3e8d4fefa5e0 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
>>>>> @@ -155,8 +155,11 @@ enum power_event {
>>>>>  #define MTL_CHAN_RX_DEBUG(x)        (MTL_CHANX_BASE_ADDR(x) + 0x38)
>>>>>
>>>>>  #define MTL_OP_MODE_RSF            BIT(5)
>>>>> +#define MTL_OP_MODE_TXQEN        BIT(3)
>>>>>  #define MTL_OP_MODE_TSF            BIT(1)
>>>>>
>>>>> +#define MTL_OP_MODE_TQS_MASK        GENMASK(24, 16)
>>>>> +
>>>>>  #define MTL_OP_MODE_TTC_MASK        0x70
>>>>>  #define MTL_OP_MODE_TTC_SHIFT        4
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>>>> index 116151cd6a95..577316de6ba8 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
>>>>> @@ -213,7 +213,17 @@ static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
>>>>>          else
>>>>>              mtl_tx_op |= MTL_OP_MODE_TTC_512;
>>>>>      }
>>>>> -
>>>>> +    /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
>>>>> +     * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
>>>>> +     * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
>>>>> +     * with reset values: TXQEN off, TQS 256 bytes.
>>>>> +     *
>>>>> +     * Write the bits in both cases, since it will have no effect when RO.
>>>>> +     * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
>>>>> +     * be RO, however, writing the whole TQS field will result in a value
>>>>> +     * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
>>>>> +     */
>>>>> +    mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
>>>> Your patch sounds good. Just one question:
>>>>
>>>> In synopsys databook I use, I see that MTL_OP_MODE_TXQEN for channel 2 can take several values "disabled / enabled / Enabled in AV mode":
>>>>
>>>> Transmit Queue Enable
>>>> This field is used to enable/disable the transmit queue 1. 00 R/W
>>>> ■ 2'b00 - Not enabled
>>>> ■ 2'b01 - Enable in AV mode (Reserved when Enable Audio Video
>>>> Bridging is not selected while configuring the core)
>>>> ■ 2'b10 - Enabled
>>>> ■ 2'b11 - Reserved
>>>>
>>>> Do you plan to manage av mode in a future patch ?
>>> We are not planning on using the AV mode.
>>> We will probably not use TXQ1 at all.
>>>
>>> I noticed that the MAC_HW_Feature2 Register actually has a TXQCNT field.
>>> It is currently saved in priv->dma_cap.number_tx_channel.
>>> If you prefer, I could do a patch V2 where we only set the bits if
>>> priv->dma_cap.number_tx_channel > 1
>>
>> Oh, sorry, that was number of tx _channels_,
>> not number of tx _queues_.
>>
>> However, we could add a number_tx_queue to struct dma_features,
>> if you would prefer that.
>
> I agree your patch is good. It will work even if we use several tx channels. We will see in the future for AV mode.

Excellent :)

Can I convert that into an Acked-by?

>
> regards
> alex
>
>>
>>>
>>> However, I don't think the current patch is too bad either, since the bits
>>> are RO when number_tx_channel == 1.
>>>
>>>
>>>> Regards
>>>> Alex
>>>>
>>>>>      writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
>>>>>
>>>>>      mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>>>>>
>>
David Miller Nov. 28, 2016, 4:29 p.m. UTC | #6
From: Niklas Cassel <niklas.cassel@axis.com>
Date: Thu, 24 Nov 2016 15:36:33 +0100

> From: Niklas Cassel <niklas.cassel@axis.com>
> 
> The dwmac4 IP can synthesized with 1-8 number of tx queues.
> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
> by default. For these IPs, the bitfield TXQEN is R/W.
> 
> Always enable tx queue 0. The write will have no effect on IPs synthesized
> with DWC_EQOS_NUM_TXQ == 1.
> 
> The driver does still not utilize more than one tx queue in the IP.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Alexandre, we are still waiting for your implicit/explicit ACK on this
change.

Thank you.
David Miller Nov. 30, 2016, 12:11 a.m. UTC | #7
From: Niklas Cassel <niklas.cassel@axis.com>
Date: Thu, 24 Nov 2016 15:36:33 +0100

> From: Niklas Cassel <niklas.cassel@axis.com>
> 
> The dwmac4 IP can synthesized with 1-8 number of tx queues.
> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
> by default. For these IPs, the bitfield TXQEN is R/W.
> 
> Always enable tx queue 0. The write will have no effect on IPs synthesized
> with DWC_EQOS_NUM_TXQ == 1.
> 
> The driver does still not utilize more than one tx queue in the IP.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Applied to net-next, thanks.
Alexandre TORGUE Dec. 1, 2016, 5:21 p.m. UTC | #8
Hi David and Niklas,

On 11/28/2016 05:29 PM, David Miller wrote:
> From: Niklas Cassel <niklas.cassel@axis.com>
> Date: Thu, 24 Nov 2016 15:36:33 +0100
>
>> From: Niklas Cassel <niklas.cassel@axis.com>
>>
>> The dwmac4 IP can synthesized with 1-8 number of tx queues.
>> On an IP synthesized with DWC_EQOS_NUM_TXQ > 1, all txqueues are disabled
>> by default. For these IPs, the bitfield TXQEN is R/W.
>>
>> Always enable tx queue 0. The write will have no effect on IPs synthesized
>> with DWC_EQOS_NUM_TXQ == 1.
>>
>> The driver does still not utilize more than one tx queue in the IP.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>
> Alexandre, we are still waiting for your implicit/explicit ACK on this
> change.

Yes you could add my Acked-by but it is already merged. My fault.
Sorry for my late answer :(

>
> Thank you.
>
diff mbox

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 6f4f5ce25114..3e8d4fefa5e0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -155,8 +155,11 @@  enum power_event {
 #define MTL_CHAN_RX_DEBUG(x)		(MTL_CHANX_BASE_ADDR(x) + 0x38)
 
 #define MTL_OP_MODE_RSF			BIT(5)
+#define MTL_OP_MODE_TXQEN		BIT(3)
 #define MTL_OP_MODE_TSF			BIT(1)
 
+#define MTL_OP_MODE_TQS_MASK		GENMASK(24, 16)
+
 #define MTL_OP_MODE_TTC_MASK		0x70
 #define MTL_OP_MODE_TTC_SHIFT		4
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 116151cd6a95..577316de6ba8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -213,7 +213,17 @@  static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
 		else
 			mtl_tx_op |= MTL_OP_MODE_TTC_512;
 	}
-
+	/* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
+	 * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
+	 * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
+	 * with reset values: TXQEN off, TQS 256 bytes.
+	 *
+	 * Write the bits in both cases, since it will have no effect when RO.
+	 * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
+	 * be RO, however, writing the whole TQS field will result in a value
+	 * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
+	 */
+	mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
 	writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
 
 	mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));