10:20 <apritzel> jernej: regarding the AC200 driver split: what about we move the regmap accessing part totally out of the PHY driver?

10:21 <apritzel> jernej: just have the PHY reference the reset controller in the MFD child device, and do the setup there (reset, LED polarity, PHY address, ...)

10:22 <apritzel> I think that would mimic the "normal" PHY setup, where this is typically done in hardware, using pins or just board layout decisions

10:24 <apritzel> PHY address and LED polarity could be MFD child DT properties, I think we need the latter anyway, since it's a board layout decision?

10:26 <apritzel> and there is already a generic resets property for PHY bindings, so we just add the few lines in the PHY driver to de-assert this reset, and this would trigger the setup on the MFD child side?

12:07 <cyrozap> apritzel: Nice work on the FEL thing! I know you're not done yet, but it's good to see you're making progress.

13:39 <apritzel> cyrozap: you bindiff'ed dram_sun50i_h616.o between GCC 10.3 and 12.1, right? Without finding anything sticking out?

15:15 <jernej> apritzel: absolutely, regmap part should be removed from PHY driver

15:16 <jernej> but it would be strange to me to specify LED polarity in AC200 EPHY node, but well, still better than comming up with some tricky alternative

15:16 <jernej> phy address should be read from PHY node, right?

15:17 <apritzel> so looking around I was thinking of having a pure MDIO PHY driver, no extra compatible, just with mdio_device_id matching, but having that "resets" property, triggering the MFD device reset controller

15:18 <apritzel> that would be basically the equivalent to the Realtek PHY driver, which is only controller via MDIO

15:19 <apritzel> its configuration (PHY address, LEDs) is typically done "in hardware" (pins tied to GND/Vcc), and the MFD child would take that role

15:19 <apritzel> you said the H3 internal PHY shares the MDIO device ID?

15:23 <jernej> yes, at least if I remember correctly

15:24 <jernej> I suspect it's completely the same PHY

15:24 <jernej> I think it would also match to A64 and H5 internal PHYs

15:24 <jernej> which is IMO good thing

15:25 <apritzel> do we match that ID already?

15:25 <jernej> no

15:26 <jernej> at least last time I checked, which is a long time ago

15:26 <apritzel> so there is some magic going on with allwinner,sun8i-h3-mdio-internal, right?

15:26 <jernej> why do you think so?

15:26 <apritzel> or allwinner,sun8i-h3-mdio-mux

15:26 <jernej> in general, PHYs should have good defaults

15:27 <apritzel> because someone must enable that clock referenced in the PHY, AFAICS this is not a generic property handled by ethernet-phy-ieee802.3-c22

15:27 <jernej> Realtek PHYs work with or without driver

15:27 <apritzel> yes, but they don't require special properties

15:28 <jernej> emac driver sets syscon register accordingly for internal PHYs, if that's what you think

15:28 <jernej> but there is no need for MDIO configuration

15:28 <jernej> at least it seems so

15:29 <apritzel> I meant that sunxi-h3-h5.dtsi references clocks and resets in the internal PHY node, and I don't think the generic PHY driver handles the clock

15:31 <apritzel> jernej: that MDIO setup that you do in ac200_ephy_config_init() (Disable APS, PHYAFE TRX optimization, ...), is that also applicable to the H3 PHYs? Or is it just setting the default values, for good measures?

15:32 <jernej> Interesting, I don't think clock property is valid, per DT bindings. Maybe montjoie will know?

15:32 <apritzel> yeah, also I didn't find it in the of_mdio driver, but reset handling is there

15:33 <jernej> apritzel: I copied (and partially rewrite) init procedure from BSP driver

15:33 <apritzel> ah, I see

15:33 <apritzel> I will have a closer look

15:33 <jernej> and I think it actually applies also for older SoCs

15:33 <jernej> I don't really remember everything anymore

15:33 <apritzel> the H616 is the same AC200 co-packaged like the H6

15:33 <apritzel> ?

15:34 <jernej> should be, yes

15:34 <jernej> IIRC, H3 BSP has basically the same PHY driver with some different hacks for probing

15:36 <apritzel> and yeah, we handle clock and reset explicitly in dwmac-sun8i.c:get_ephy_nodes()

15:37 <apritzel> by scanning child nodes ...

15:37 <jernej> IIRC there was a big discussion how to properly represent everything in DT

15:38 <apritzel> yeah, I dimly remember ...

15:44 <apritzel> and I think the PHY address, to be programmed into the I2C register, must come from the DT, next to the LED polarity

15:44 <apritzel> because you could have multiple AC200s in your system

15:47 <apritzel> also as you mentioned A64 above: that doesn't have an internal PHY, right? It's just only H3 and H5, and later SoCs (H6, H616) used the co-packaged AC200

15:52 <jernej> ah, right

22:35 <cyrozap> apritzel: dram_sun50i_h616.o only contains three functions, since a lot of them were declared static and simply got included in `mctl_core_init`. The bulk of the differences are in `mctl_core_init`, with the remaining two functions (`sunxi_dram_init` and `mbus_configure_port`) having negligible differences (`mbus_configure_port` is nearly identical, with some instructions moved around and tst/csel

22:35 <cyrozap> instructions removed, and `sunxi_dram_init` has a small number of similarly-inconsequential optimizations).

22:43 <cyrozap> One of the static functions contained in `mctl_core_init` (`mctl_phy_read_training`) is included by one of the configure flags you mentioned earlier ("but when I disable CONFIG_DRAM_SUN50I_H616_READ_TRAINING, it works with both compilers"), so either there's a problem with that function and disabling that one function fixed the return to FEL, or disabling CONFIG_DRAM_SUN50I_H616_WRITE_LEVELING,

22:43 <cyrozap> CONFIG_DRAM_SUN50I_H616_READ_TRAINING, and CONFIG_DRAM_SUN50I_H616_WRITE_TRAINING together is what fixed it.

22:45 <cyrozap> So, maybe it really is an issue with the stack, like you mentioned earlier--I'm thinking if those functions were disabled, it might free up enough stack space to avoid clobbering the saved mask ROM data.

22:54 <apritzel> cyrozap: yeah, the stack idea is really tempting, as it would explain everything nicely:

22:55 <apritzel> GCC 12 uses a bit more stack, so something gets overwritten which was not touched before

22:56 <apritzel> and including more functions also increases the stack usage, which would explain why the X96 Mate works, as it doesn't use most of those "extra" functions

22:56 <apritzel> the only problem is: it cannot be the stack (alone), as moving it away doesn't fix it

23:05 <cyrozap> apritzel: Yeah, and I'm comparing the assembly listings of the working and broken functions and they both appear to use the same amount of stack space...

23:06 <apritzel> exactly, I checked that as well

23:09 <apritzel> another thing I am trying is to move the *later* SPL stack into SRAM as well. After DRAM init, we switch BSS and stack to the newly initialised DRAM. So if DRAM is not initialised correctly, it might explain the failure

23:25 <apritzel> cyrozap: mmh, no change :-(

23:33 <apritzel> cyrozap: and yeah, the inlining is slightly annoying (though clever). I identified and matched each C line from mctl_phy_read_training() in the 12.1 objdump, but don't see any obvious issues

23:37 <cyrozap> apritzel: Is there a way to get JTAG access on these chips? Maybe attaching a debugger would at least let you see where in the return-to-FEL process things are getting stuck, or let you dump memory at that point in the broken binary so you can compare it to the memory at the same point during execution of the working binary.

23:39 <smaeul> > moving it away doesn't fix it // which did you move, the SPL stack or the save area?

23:39 <smaeul> I woulder if bisecting GCC optimization flags would be a useful way of debugging this

23:40 <smaeul> cyrozap: you can get JTAG on port F with a microSD breakout board