| Message ID | 20250204161359.3335241-1-wens@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [netdev] net: stmmac: dwmac-rk: Provide FIFO sizes for DWMAC 1000 | expand |
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
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.
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.
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.
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.
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
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);
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);