Message-ID: <c18fbdf0-b8e2-4fa7-8bb7-a3fed8960970@leica-geosystems.com>
Date: Wed, 14 Jan 2026 14:09:27 +0000
From: SHUKLA Mamta <mamta.shukla@...ca-geosystems.com>
To: Dinh Nguyen <dinguyen@...nel.org>, "Russell King (Oracle)"
	<linux@...linux.org.uk>
CC: Maxime Chevallier <maxime.chevallier@...tlin.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre
 Torgue <alexandre.torgue@...s.st.com>, Philipp Zabel
	<p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Ahmad Fatoum
	<a.fatoum@...gutronix.de>, GEO-CHHER-bsp-development
	<bsp-development.geo@...ca-geosystems.com>, Pengutronix Kernel Team
	<kernel@...gutronix.de>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] net: stmmac: socfpga: add call to assert/deassert
 ahb reset line

Hello Dinh,

On 1/13/26 22:37, Dinh Nguyen wrote:
>
> On 1/8/26 10:10, Russell King (Oracle) wrote:
>> On Thu, Jan 08, 2026 at 07:08:09AM -0600, Dinh Nguyen wrote:
>>> The "stmmaceth-ocp" reset line of stmmac controller on the SoCFPGA
>>> platform is essentially the "ahb" reset on the standard stmmac
>>> controller. But since stmmaceth-ocp has already been introduced into
>>> the wild, we cannot just remove support for it. But what we can do is
>>> to support both "stmmaceth-ocp" and "ahb" reset names. Going forward we
>>> will be using "ahb", but in order to not break ABI, we will be call 
>>> reset
>>> assert/de-assert both ahb and stmmaceth-ocp.
>>>
>>> The ethernet hardware on SoCFPGA requires either the stmmaceth-ocp or
>>> ahb reset to be asserted every time before changing the phy mode, then
>>> de-asserted when the phy mode has been set.
>>
>> This is not SoCFPGA specific. The dwmac core only samples its
>> phy_intf_sel_i signals when coming out of reset, and then latches
>> that as the operating mode.
>>
>> Currently, the dwmac core driver does not support dynamically changing
>> plat_dat->phy_interface at runtime. That may change in the future, but
>> as it requires a hardware reset which will clear out the PTP state, it
>> would need consideration of that effect.
>>
>> The SoCFPGA driver only calls the set_phy_mode() methods from
>> socfpga_dwmac_init(), which in turn is called from the plat_dat->init
>> hook. This will be called from:
>>
>> 1. When stmmac_dvr_probe() is called, prior to allocating any
>>     resources, and prior to the core driver's first call to:
>>     reset_control_deassert(priv->plat->stmmac_ahb_rst);
>>
>> 2. As plat_dat->resume is not populated by the glue driver, the init
>>     hook will also be called when resuming from stmmac_resume().
>>
>> Lastly, nothing in the main driver corrently writes to ->phy_interface.
>>
>> I would like to see the platform glue drivers using more of what is
>> in the core driver, rather than re-inventing it, so I support the
>> idea of getting rid of dwmac->stmmac_ocp_rst.
>>
>> What I suggest is to get rid of dwmac->stmmac_ocp_rst now.
>> devm_stmmac_probe_config_dt() will parse the device tree, looking for
>> the "ahb" reset, and assigning that to plat->stmmac_ahb_rst. If it
>> doesn't exist, then plat->stmmac-ahb_rst will be NULL.
>>
>> So, in socfpga_dwmac_probe(), do something like this:
>>
>>       struct reset_control *ocp_rst;
>> ...
>>       if (!plat_dat->stmmac_ahb_rst) {
>>               ocp_rst = devm_reset_control_get_optional(dev, 
>> "stmmaceth-ocp");
>>               if (IS_ERR(ocp_rst))
>>                       return dev_err_probe(dev, PTR_ERR(ocp_rst),
>>                                            "failed to get ocp reset");
>>
>>               if (ocp_rst)
>>                       dev_warn(dev, "ocp reset is deprecated, please 
>> update device tree.\n");
>>
>>               plat_dat->stmmac_ahb_rst = ocp_rst;
>>       }
>>
>> Then, change all remaining instances of dwmac->stmmac_ocp_rst to
>> dwmac->plat_dat->stmmac_ahb_rst... and job done. You have compatibility
>> with device trees that use "ahb", and with device trees that use
>> "stmmaceth-ocp".
>>
>> Given that struct socfpga_dwmac contains the plat_dat pointer, rather
>> than copying plat_dat->stmmac_rst to your private structure, please
>> use the one in the plat_dat structure.
>>
>> The next question I have is - do you need to assert both the AHB reset
>> and stmmac_rst to set the PHY interface mode? I don't see a dependency
>> between these two resets in the socfpga code - the driver doesn't treat
>> them as nested. It asserts the AHB reset _then_ the stmmac reset, and
>> then releases them in the same order rather than reverse order. This
>> suggests there's no interdependence between them, and probably it's
>> only necessary to assert the stmmac core's reset (stmmac_rst).
>>
>> So, maybe the driver can leave the handling of plat_dat->stmmac_ahb_rst
>> to the stmmac core code?
>>
>
> Thanks for the suggestion. According to this commit[1], it seems that
> both reset lines need to get toggled. But I'm going to run some test on
> HW and make the appropriate changes.

On Arria10 socfpga, both reset lines i.e stmmac_ocp_rst and stmmac_rst 
are needed since EMAC Controller on Arria10 supports Tx Rx FIFO with ECC

RAM and as per datasheet[1]:

`An EMAC ECC RAM reset asserts a reset to both the memory and the 
multiplexed EMAC bus interface clock, ap_clk. You should ensure that 
both the EMAC ECC RAM and the EMAC Module resets are deasserted before 
beginning transactions. Program the emac*ocp bits and the emac* bits in 
the per0modrst register of the Reset Manager to deassert reset in the 
EMAC's ECC RAM and the EMAC module, respectively.`

[1]https://docs.altera.com/r/docs/683711/22.3/intel-arria-10-hard-processor-system-technical-reference-manual/taking-the-ethernet-mac-out-of-reset


OTOH, we can't have both scp and ahb rst together while setting phy 
mode, since they are basically same reset lines and having both leads to 
warning:

[    0.742218] CAN device driver interface
[    0.746401] socfpga-dwmac ff800000.ethernet: IRQ eth_wake_irq not found
[    0.753018] socfpga-dwmac ff800000.ethernet: IRQ eth_lpi not found
[    0.759177] socfpga-dwmac ff800000.ethernet: IRQ sfty not found
[    0.765159] socfpga-dwmac ff800000.ethernet: Deprecated MDIO bus 
assumption used
[    0.772674] ------------[ cut here ]------------
[    0.777278] WARNING: /drivers/reset/core.c:457 at 
reset_control_assert+0x178/0x1f8, CPU#0: swapper/0/1
[    0.786585] Modules linked in:
[    0.789638] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 
6.19.0-rc3-00262-g36d53f861448 #1 PREEMPT
[    0.789652] Hardware name: Altera SOCFPGA Arria10
[    0.789657] Call trace:
[    0.789666]  unwind_backtrace from show_stack+0x18/0x1c
[    0.789688]  show_stack from dump_stack_lvl+0x54/0x68
[    0.789703]  dump_stack_lvl from __warn+0x98/0x180
[    0.789719]  __warn from warn_slowpath_fmt+0x1bc/0x1c4
[    0.789733]  warn_slowpath_fmt from reset_control_assert+0x178/0x1f8
[    0.789749]  reset_control_assert from 
socfpga_gen10_set_phy_mode+0xec/0x1f4
[    0.789776]  socfpga_gen10_set_phy_mode from stmmac_dvr_probe+0x40/0x1154
[    0.789804]  stmmac_dvr_probe from devm_stmmac_pltfr_probe+0x34/0xa8
[    0.789825]  devm_stmmac_pltfr_probe from socfpga_dwmac_probe+0x160/0x1b8
[    0.789847]  socfpga_dwmac_probe from platform_probe+0x64/0x98
[    0.789875]  platform_probe from really_probe+0xc0/0x2bc
[    0.789896]  really_probe from __driver_probe_device+0x90/0x1a4
[    0.789913]  __driver_probe_device from driver_probe_device+0x38/0x10c
[    0.789931]  driver_probe_device from __driver_attach+0x98/0x180
[    0.789948]  __driver_attach from bus_for_each_dev+0x84/0xd4
[    0.789965]  bus_for_each_dev from bus_add_driver+0xd4/0x1f4
[    0.789980]  bus_add_driver from driver_register+0x84/0x11c
[    0.789998]  driver_register from do_one_initcall+0x60/0x270
[    0.790015]  do_one_initcall from kernel_init_freeable+0x228/0x294
[    0.790036]  kernel_init_freeable from kernel_init+0x24/0x138
[    0.790055]  kernel_init from ret_from_fork+0x14/0x28
[    0.790069] Exception stack(0xd202dfb0 to 0xd202dff8)
[    0.790078] dfa0:                                     00000000 
00000000 00000000 00000000
[    0.790087] dfc0: 00000000 00000000 00000000 00000000 00000000 
00000000 00000000 00000000
[    0.790094] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.790100] ---[ end trace 0000000000000000 ]---
[    0.963016] socfpga-dwmac ff800000.ethernet: User ID: 0x10, Synopsys 
ID: 0x37
[    0.970148] socfpga-dwmac ff800000.ethernet:     DWMAC1000
[    0.976025] socfpga-dwmac ff800000.ethernet: DMA HW capability 
register supported
[    0.983511] socfpga-dwmac ff800000.ethernet: RX Checksum Offload 
Engine supported

>
> Dinh
>
> [1]
> https://lore.kernel.org/all/20250205134428.529625725@linuxfoundation.org/

---

Mamta

Powered by blists - more mailing lists