diff mbox series

[net,2/2] net: stmmac: Limit the number of MTL queues to maximum value

Message ID 20250116020853.2835521-2-hayashi.kunihiko@socionext.com (mailing list archive)
State New
Headers show
Series [net,1/2] net: stmmac: Limit FIFO size by hardware feature value | expand

Commit Message

Kunihiko Hayashi Jan. 16, 2025, 2:08 a.m. UTC
The number of MTL queues to use is specified by the parameter
"snps,{tx,rx}-queues-to-use" from the platform layer.

However, the maximum number of queues is determined by
the macro MTL_MAX_{TX,RX}_QUEUES. It's appropriate to limit the
values not to exceed the upper limit values.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Furong Xu Jan. 16, 2025, 3:28 a.m. UTC | #1
On Thu, 16 Jan 2025 11:08:53 +0900, Kunihiko Hayashi <hayashi.kunihiko@socionext.com> wrote:

> The number of MTL queues to use is specified by the parameter
> "snps,{tx,rx}-queues-to-use" from the platform layer.
> 
> However, the maximum number of queues is determined by
> the macro MTL_MAX_{TX,RX}_QUEUES. It's appropriate to limit the
> values not to exceed the upper limit values.
> 

The Fixes: tag is required too.

> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index ad868e8d195d..471eb1a99d90 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -165,6 +165,8 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
>  	if (of_property_read_u32(rx_node, "snps,rx-queues-to-use",
>  				 &plat->rx_queues_to_use))
>  		plat->rx_queues_to_use = 1;
> +	if (plat->rx_queues_to_use > MTL_MAX_RX_QUEUES)
> +		plat->rx_queues_to_use = MTL_MAX_RX_QUEUES;

MTL_MAX_RX_QUEUES, MTL_MAX_TX_QUEUES and STMMAC_CH_MAX are defined to 8,
this is correct for gmac4, but xgmac has 16 channels at most.

Drop these legacy defines and always use
priv->dma_cap.number_rx_queues,
priv->dma_cap.number_tx_queues,
priv->dma_cap.number_tx_channel,
priv->dma_cap.number_rx_channel,
seems like a good option.

>  
>  	if (of_property_read_bool(rx_node, "snps,rx-sched-sp"))
>  		plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
> @@ -224,6 +226,8 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
>  	if (of_property_read_u32(tx_node, "snps,tx-queues-to-use",
>  				 &plat->tx_queues_to_use))
>  		plat->tx_queues_to_use = 1;
> +	if (plat->tx_queues_to_use > MTL_MAX_TX_QUEUES)
> +		plat->tx_queues_to_use = MTL_MAX_TX_QUEUES;
>  
>  	if (of_property_read_bool(tx_node, "snps,tx-sched-wrr"))
>  		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_WRR;
Kunihiko Hayashi Jan. 16, 2025, 10:02 a.m. UTC | #2
Hi Furong,

Thank you for your comment.

On 2025/01/16 12:28, Furong Xu wrote:
> On Thu, 16 Jan 2025 11:08:53 +0900, Kunihiko Hayashi
> <hayashi.kunihiko@socionext.com> wrote:
> 
>> The number of MTL queues to use is specified by the parameter
>> "snps,{tx,rx}-queues-to-use" from the platform layer.
>>
>> However, the maximum number of queues is determined by
>> the macro MTL_MAX_{TX,RX}_QUEUES. It's appropriate to limit the
>> values not to exceed the upper limit values.
>>
> 
> The Fixes: tag is required too.

I see. I'll find it.

>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index ad868e8d195d..471eb1a99d90 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -165,6 +165,8 @@ static int stmmac_mtl_setup(struct platform_device
> *pdev,
>>   	if (of_property_read_u32(rx_node, "snps,rx-queues-to-use",
>>   				 &plat->rx_queues_to_use))
>>   		plat->rx_queues_to_use = 1;
>> +	if (plat->rx_queues_to_use > MTL_MAX_RX_QUEUES)
>> +		plat->rx_queues_to_use = MTL_MAX_RX_QUEUES;
> 
> MTL_MAX_RX_QUEUES, MTL_MAX_TX_QUEUES and STMMAC_CH_MAX are defined to 8,
> this is correct for gmac4, but xgmac has 16 channels at most.

Yes, but these macros are used as the number of elements in some arrays.
(used in common.h and stmmac.h)

We can change these value to the maximum (16), though, the size of some
structures will increase so be careful.

> Drop these legacy defines and always use
> priv->dma_cap.number_rx_queues,
> priv->dma_cap.number_tx_queues,
> priv->dma_cap.number_tx_channel,
> priv->dma_cap.number_rx_channel,
> seems like a good option.

These values can be obtained in stmmac_dvr_probe() and this patch checks
the queue sizes before stmmac_dvr_probe(). I think we need to change the
way how to check them.

And when checking only with dma_cap values, the number of elements in 
the arrays might be exceeded.  It is necessary to take care of both the
arrays and the capabilities.

Thank you,

---
Best Regards
Kunihiko Hayashi
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index ad868e8d195d..471eb1a99d90 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -165,6 +165,8 @@  static int stmmac_mtl_setup(struct platform_device *pdev,
 	if (of_property_read_u32(rx_node, "snps,rx-queues-to-use",
 				 &plat->rx_queues_to_use))
 		plat->rx_queues_to_use = 1;
+	if (plat->rx_queues_to_use > MTL_MAX_RX_QUEUES)
+		plat->rx_queues_to_use = MTL_MAX_RX_QUEUES;
 
 	if (of_property_read_bool(rx_node, "snps,rx-sched-sp"))
 		plat->rx_sched_algorithm = MTL_RX_ALGORITHM_SP;
@@ -224,6 +226,8 @@  static int stmmac_mtl_setup(struct platform_device *pdev,
 	if (of_property_read_u32(tx_node, "snps,tx-queues-to-use",
 				 &plat->tx_queues_to_use))
 		plat->tx_queues_to_use = 1;
+	if (plat->tx_queues_to_use > MTL_MAX_TX_QUEUES)
+		plat->tx_queues_to_use = MTL_MAX_TX_QUEUES;
 
 	if (of_property_read_bool(tx_node, "snps,tx-sched-wrr"))
 		plat->tx_sched_algorithm = MTL_TX_ALGORITHM_WRR;