Created attachment 4458 [details] Suggested solution code patch Preconditions/Environment ------------------------- We use imx6-solox processor and Marvel 88E6390 switch to design a special device, we want to migrate linux OS to this device, but when we add Marvel’s SDK to linux kernel with standard dts file, we found that the Cpu was hang during the startup of linux OS, after we analyzed this phenomenon, we think that’s a linux kernel bug and confirm that the BUG exists in version 4.9.11 and 4.19.2. Triggering Action/Cause ----------------------- In the process of analyzing the problem, we found that if we read/write enet module’s registers when enet clock(CCM_CCGR3, bit 5-4) is disabled, the processor will hang. But after we analyzed the kernel code, we speculate that this is caused by chip design, and the designer wish to circumvent problems through software, because we found that in the read/write functions of the source file fec_main.c, the designer always resume the enet clock before read/write registers. But the kernel code of imx6-solox platform would cause a special environment which can bypass the clock management mechanism mentioned in the previous article, in this environment if some task read/write enet registers, the CPU hang will occur. Now I will explain how the environment appears. In the source file clk-imx6sx.c, the Array variable clks define two enet module’s clk member variable ‘enet and ‘enet-ahb’, actually these two clks point to the same register (CCM_CCGR3, bit 5-4 enet clock). In dts file, fec device’s clock ‘ipg’ point to ‘enet’ and ‘ahb’ point to ‘enet-ahb’, it means that we modify any one clk will affect other. The source file fec_main.c is the generic code for freescale’s product, It contains all the enet port code, in the device probe function fec_probe(), firstly it will get several clk by name from the dts and enable all clks,in the end of fec_probe(),each clk will be disabled directly except for ‘ipg’ clk, the ‘ipg’ clk will be disabled by kernel’s resume/suspend mechanism if we turn on power saving, or the ‘ipg’ clk will be enabled. But fec_probe() would disable ‘ipg’ clk directly when ‘ahb’ clk is disabled because they point to the same enet clock register. Finally after fec_probe() function the enet module’ clk will be disabled no matter power saving is enabled or disabled. The power saving related modules will affect the BUG, So we will discuss the two situations of turning on or off the power saving. 1)Turn off power saving Turn off power saving means that the kernel’s resume/suspend mechanism is disabled, so if we use read/write function offer by fec_main.c, it can’t resume enet clock, the CPU hang will occur within the time range from when the device is probed until we open the device. We think that’s a BUG, because read/write enet register after enet device has been probed is a common operation, for example, we can registered a switch or phy device under enet’s mdio bus, this is a common way to use, when device probe we create a poll task to monitor irq or any other register value, then the CPU will immediately hang. 2)Turn on power saving Turn on power saving is a more complicated environment because the kernel’s resume/suspend mechanism is enabled. After kernel startup, kernel will create two clk node for ‘enet’ and ‘enet-ahb’, the clk node will save the enable state of clk, so if we modify one of the two clks individually will cause the clk’s state and actual register value do not match, this’s why the BUG appears. The fec_probe() will disble enet clock, the read/write functions will resume ‘ipg’ clk before read/write register, then use auto-suspend to disable ‘ipg’ clks. Under normal circumstances, this mechanism can guarantee the normal operation of reading and writing, but if a task read/write enet register at a high frequently, the auto-suspend would never timeout to enter the suspend function. At this time, the ‘enet’ clk node’s state is the same as register value, but the ‘enet-ahb’ clk node’s state is different with register value because enable ‘ipg’ clk change the register value. In kernel_init() function, it will call the kernel’s clk late initcall function clk_disable_unused() , which can traversing the clk node to check whether the clk is unused. At this time, the ‘enet-ahb’ clk node’s state and actual register value do not match will be treated as unused clk, then the ‘enet-ahb’ clk will be diabled, it means the enet clock register will be disable. But the resume/suspend mechanism didn’t know the register value is modified, so the resume operation before read/write enet module’s register won’t really modify the enet clock register value because the auto-suspend never timeout. Then the task will read/write enet module’s register when the enet clock has been disable, the CPU will hang. Expectation ----------- linux OS successfully started Actual Result ------------- the CPU will hang Reproducibility --------------- 100% happen Suggested solution --------------- To avoid modifying generic code, our solution is modify platform related source code file: clk-imx6sx.c, we suggest that delete ‘enet-ahb’ member variable in the array variable clks. Then modify fec device’s clocks in the dts file, point ‘ahb’ from IMX6SX_CLK_ENET_AHB to IMX6SX_CLK_ENET. The code patch is in the attachment.
Hi Kay, Your analysis seems to be correct. Could you please post a formal patch (only the dts part) against 5.1-rc3 and submit it to the people and lists shown by ./scripts/get_maintainer.pl your.patch? Thanks
I have tried to post patch(created by diff) to the people and lists shown by ./scripts/get_maintainer.pl directly, but a maintainer told me that's not a formal process and what's the formal process, I will resend patch later.
I have already post the patch to the people and lists shown by ./scripts/get_maintainer.pl, if the email has any problems, please tell me.
Hi, We;'ve agreed FSL bugs should now be moved to their github issues. If you're still having this problem and it wasn't resolved, please open an issue on github.