[v1] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

[PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Chen-Yu Tsai 10 months, 1 week ago

From: Chen-Yu Tsai <wens@csie.org>

The DWMAC 1000 DMA capabilities register does not provide actual
FIFO sizes, nor does the driver really care. If they are not
provided via some other means, the driver will work fine, only
disallowing changing the MTU setting.

The recent commit 8865d22656b4 ("net: stmmac: Specify hardware
capability value when FIFO size isn't specified") changed this by
requiring the FIFO sizes to be provided, breaking devices that were
working just fine.

Provide the FIFO sizes through the driver's platform data, to not
only fix the breakage, but also enable MTU changes. The FIFO sizes
are confirmed to be the same across RK3288, RK3328, RK3399 and PX30,
based on their respective manuals. It is likely that Rockchip
synthesized their DWMAC 1000 with the same parameters on all their
chips that have it.

Fixes: eaf4fac47807 ("net: stmmac: Do not accept invalid MTU values")
Fixes: 8865d22656b4 ("net: stmmac: Specify hardware capability value when FIFO size isn't specified")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
The reason for stable inclusion is not to fix the device breakage
(which only broke in v6.14-rc1), but to provide the values so that MTU
changes can work in older kernels.

Since a fix for stmmac in general has already been sent [1] and a revert
was also proposed [2], I'll refrain from sending mine.

[1] https://lore.kernel.org/all/20250203093419.25804-1-steven.price@arm.com/
[2] https://lore.kernel.org/all/Z6Clkh44QgdNJu_O@shell.armlinux.org.uk/

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index a4dc89e23a68..71a4c4967467 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1966,8 +1966,11 @@ static int rk_gmac_probe(struct platform_device *pdev)
 	/* If the stmmac is not already selected as gmac4,
 	 * then make sure we fallback to gmac.
 	 */
-	if (!plat_dat->has_gmac4)
+	if (!plat_dat->has_gmac4) {
 		plat_dat->has_gmac = true;
+		plat_dat->rx_fifo_size = 4096;
+		plat_dat->tx_fifo_size = 2048;
+	}
 	plat_dat->fix_mac_speed = rk_fix_speed;
 
 	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
-- 
2.39.5

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Steven Price 10 months, 1 week ago

On 04/02/2025 16:13, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The DWMAC 1000 DMA capabilities register does not provide actual
> FIFO sizes, nor does the driver really care. If they are not
> provided via some other means, the driver will work fine, only
> disallowing changing the MTU setting.
> 
> The recent commit 8865d22656b4 ("net: stmmac: Specify hardware
> capability value when FIFO size isn't specified") changed this by
> requiring the FIFO sizes to be provided, breaking devices that were
> working just fine.
> 
> Provide the FIFO sizes through the driver's platform data, to not
> only fix the breakage, but also enable MTU changes. The FIFO sizes
> are confirmed to be the same across RK3288, RK3328, RK3399 and PX30,
> based on their respective manuals. It is likely that Rockchip
> synthesized their DWMAC 1000 with the same parameters on all their
> chips that have it.
> 
> Fixes: eaf4fac47807 ("net: stmmac: Do not accept invalid MTU values")
> Fixes: 8865d22656b4 ("net: stmmac: Specify hardware capability value when FIFO size isn't specified")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

I think it's better at this stage to apply the revert first. However
I've run this on my board (Firefly RK3288) and it works, so when rebased
onto the (reverted) revert:

Tested-by: Steven Price <steven.price@arm.com>

Thanks,
Steve

> ---
> The reason for stable inclusion is not to fix the device breakage
> (which only broke in v6.14-rc1), but to provide the values so that MTU
> changes can work in older kernels.
> 
> Since a fix for stmmac in general has already been sent [1] and a revert
> was also proposed [2], I'll refrain from sending mine.
> 
> [1] https://lore.kernel.org/all/20250203093419.25804-1-steven.price@arm.com/
> [2] https://lore.kernel.org/all/Z6Clkh44QgdNJu_O@shell.armlinux.org.uk/
> 
>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index a4dc89e23a68..71a4c4967467 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -1966,8 +1966,11 @@ static int rk_gmac_probe(struct platform_device *pdev)
>  	/* If the stmmac is not already selected as gmac4,
>  	 * then make sure we fallback to gmac.
>  	 */
> -	if (!plat_dat->has_gmac4)
> +	if (!plat_dat->has_gmac4) {
>  		plat_dat->has_gmac = true;
> +		plat_dat->rx_fifo_size = 4096;
> +		plat_dat->tx_fifo_size = 2048;
> +	}
>  	plat_dat->fix_mac_speed = rk_fix_speed;
>  
>  	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Jakub Kicinski 10 months, 1 week ago

On Wed,  5 Feb 2025 00:13:59 +0800 Chen-Yu Tsai wrote:
> Since a fix for stmmac in general has already been sent [1] and a revert
> was also proposed [2], I'll refrain from sending mine.

No, no, please do. You need to _submit_ the revert like a normal patch.
With all the usual details in the commit message.

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Chen-Yu Tsai 10 months, 1 week ago

On Wed, Feb 5, 2025 at 5:43 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed,  5 Feb 2025 00:13:59 +0800 Chen-Yu Tsai wrote:
> > Since a fix for stmmac in general has already been sent [1] and a revert
> > was also proposed [2], I'll refrain from sending mine.
>
> No, no, please do. You need to _submit_ the revert like a normal patch.
> With all the usual details in the commit message.

Mine isn't a revert, but simply downgrading the error to a warning.
So... yet another workaround approach.

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Simon Horman 10 months, 1 week ago

On Wed, Feb 05, 2025 at 11:45:17AM +0800, Chen-Yu Tsai wrote:
> On Wed, Feb 5, 2025 at 5:43 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed,  5 Feb 2025 00:13:59 +0800 Chen-Yu Tsai wrote:
> > > Since a fix for stmmac in general has already been sent [1] and a revert
> > > was also proposed [2], I'll refrain from sending mine.
> >
> > No, no, please do. You need to _submit_ the revert like a normal patch.
> > With all the usual details in the commit message.
> 
> Mine isn't a revert, but simply downgrading the error to a warning.
> So... yet another workaround approach.

I think the point is that someone needs to formally
submit the revert. And I assume it should target the net tree.

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Russell King (Oracle) 10 months, 1 week ago

On Wed, Feb 05, 2025 at 05:38:24PM +0000, Simon Horman wrote:
> On Wed, Feb 05, 2025 at 11:45:17AM +0800, Chen-Yu Tsai wrote:
> > On Wed, Feb 5, 2025 at 5:43 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > On Wed,  5 Feb 2025 00:13:59 +0800 Chen-Yu Tsai wrote:
> > > > Since a fix for stmmac in general has already been sent [1] and a revert
> > > > was also proposed [2], I'll refrain from sending mine.
> > >
> > > No, no, please do. You need to _submit_ the revert like a normal patch.
> > > With all the usual details in the commit message.
> > 
> > Mine isn't a revert, but simply downgrading the error to a warning.
> > So... yet another workaround approach.
> 
> I think the point is that someone needs to formally
> submit the revert. And I assume it should target the net tree.

For what I think is the third time today (fourth if you include the
actual patch...)

https://lore.kernel.org/r/E1tfeyR-003YGJ-Gb@rmk-PC.armlinux.org.uk

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Simon Horman 10 months, 1 week ago

On Thu, Feb 06, 2025 at 01:40:17AM +0800, Chen-Yu Tsai wrote:
> On Thu, Feb 6, 2025 at 1:38 AM Simon Horman <horms@kernel.org> wrote:

...

> > I think the point is that someone needs to formally
> > submit the revert. And I assume it should target the net tree.
> 
> Russell sent one a couple hours ago, so I think we're covered.
> 

On Wed, Feb 05, 2025 at 05:44:58PM +0000, Russell King (Oracle) wrote:
> On Wed, Feb 05, 2025 at 05:38:24PM +0000, Simon Horman wrote:

...

> > I think the point is that someone needs to formally
> > submit the revert. And I assume it should target the net tree.
> 
> For what I think is the third time today (fourth if you include the
> actual patch...)
> 
> https://lore.kernel.org/r/E1tfeyR-003YGJ-Gb@rmk-PC.armlinux.org.uk

Thanks, I see it now.

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Chen-Yu Tsai 10 months, 1 week ago

On Thu, Feb 6, 2025 at 1:38 AM Simon Horman <horms@kernel.org> wrote:
>
> On Wed, Feb 05, 2025 at 11:45:17AM +0800, Chen-Yu Tsai wrote:
> > On Wed, Feb 5, 2025 at 5:43 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > On Wed,  5 Feb 2025 00:13:59 +0800 Chen-Yu Tsai wrote:
> > > > Since a fix for stmmac in general has already been sent [1] and a revert
> > > > was also proposed [2], I'll refrain from sending mine.
> > >
> > > No, no, please do. You need to _submit_ the revert like a normal patch.
> > > With all the usual details in the commit message.
> >
> > Mine isn't a revert, but simply downgrading the error to a warning.
> > So... yet another workaround approach.
>
> I think the point is that someone needs to formally
> submit the revert. And I assume it should target the net tree.

Russell sent one a couple hours ago, so I think we're covered.

Re: [PATCH netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000

Posted by Andrew Lunn 10 months, 1 week ago

On Wed, Feb 05, 2025 at 12:13:59AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The DWMAC 1000 DMA capabilities register does not provide actual
> FIFO sizes, nor does the driver really care. If they are not
> provided via some other means, the driver will work fine, only
> disallowing changing the MTU setting.
> 
> The recent commit 8865d22656b4 ("net: stmmac: Specify hardware
> capability value when FIFO size isn't specified") changed this by
> requiring the FIFO sizes to be provided, breaking devices that were
> working just fine.
> 
> Provide the FIFO sizes through the driver's platform data, to not
> only fix the breakage, but also enable MTU changes. The FIFO sizes
> are confirmed to be the same across RK3288, RK3328, RK3399 and PX30,
> based on their respective manuals. It is likely that Rockchip
> synthesized their DWMAC 1000 with the same parameters on all their
> chips that have it.
> 
> Fixes: eaf4fac47807 ("net: stmmac: Do not accept invalid MTU values")
> Fixes: 8865d22656b4 ("net: stmmac: Specify hardware capability value when FIFO size isn't specified")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> The reason for stable inclusion is not to fix the device breakage
> (which only broke in v6.14-rc1), but to provide the values so that MTU
> changes can work in older kernels.

Allowing the MTU to be changed is probably classed as a new feature,
not bug fix.

I _think_ this also allows flow control, which again is a new feature.

Please submit to net-next.

	Andrew