RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock

[Aisheng Dong]

> From: Fabio Estevam [mailto:festevam@xxxxxxxxx]
> Sent: Saturday, May 4, 2019 7:04 PM
> 
> Hi Kay-Liu,
> 
> On Thu, Apr 25, 2019 at 8:09 AM <liuk@xxxxxxxxxxxx> wrote:
> >
> > From: Kay-Liu <liuk@xxxxxxxxxxxx>
> >
> > The imx6sx's dts file defines five clocks for fec, the 'ahb'clock's
> > value is IMX6SX_CLK_ENET_AHB, but in the i.MX6SX Reference Manual
> > there is no such enet ahb clock, there is only one "enet clock" in the
> > CCM_CCGR3 register which is controlled by bits 5-4, the enet clock is
> > defined for the 'ipg' clock, this can cause problem.
> > The original phenomenon is using imx6-solox processor and Marvel
> > 88E6390 switch with linux OS, the kernel will hang during the startup
> > of the linux OS.
> > After analyzing the phenomenon, the reason of CPU hang is read/write
> > enet module's register when the enet clock is disabled. The kernel
> > code try to avoids the problem by resume enet clock before read/write
> > enet register.
> > But the enet module's clock config will cause a special environment
> > which can bypass the clock resume mechanism.
> > The CPU has only one enet clock, after kernel parses the dts file, the
> > two clock variables 'ipg' and 'ahb'
> > finnaly point to the same enet clock register. This will cause enet
> > clock be disabled after fec probe over.
> > Because the power saving module will affect the BUG, so there are two
> > situations for this problem:
> > 1)Turn off power saving
> > Turn off power saving means that the resume mechanism is disabled, so
> > after fec probe over if any one read/write enet module's register, the
> > CPU will hang because no one could resume the enet clock.
> > 2)Turn on power saving
> > Turn on power saving could resume enet clock before read/write enet
> > register by enable 'ipg' clk, this will cause 'ahb' variable state and
> > enet clock register value don't match.If any task read/write enet at a
> > high frequently, the kernel will keep resume state and never enter
> > suspend process, this means that the kernel will only modifies the
> > register value during the first resume.
> > But the kernel init will check unused clock variable in the late
> > initcall, the 'ahb' clock will be treated as unused, at this time, the
> > enet clock will be disabled bypass the resume mechanism, then the next
> > read/write enet module's register will cause the CPU hang.
> > Proposed solution is delete the 'ahb' clock's definition in the
> > clk-imx6sx.c, and modify fec deviceâs clocks in the dts file, point
> > âahbâ from IMX6SX_CLK_ENET_AHB to IMX6SX_CLK_ENET
> >
> > Signed-off-by: Kay-Liu <liuk@xxxxxxxxxxxx>
> > ---
> > Change since v1:
> > -inproved commit log description
> > -add platform related clock change instead of describe is in the
> > external URL
> >
> >  arch/arm/boot/dts/imx6sx.dtsi | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx6sx.dtsi
> > b/arch/arm/boot/dts/imx6sx.dtsi index 5b16e65..b8b23a6 100644
> > --- a/arch/arm/boot/dts/imx6sx.dtsi
> > +++ b/arch/arm/boot/dts/imx6sx.dtsi
> > @@ -919,7 +919,7 @@
> >                                 interrupts = <GIC_SPI 118
> IRQ_TYPE_LEVEL_HIGH>,
> >                                              <GIC_SPI 119
> IRQ_TYPE_LEVEL_HIGH>;
> >                                 clocks = <&clks IMX6SX_CLK_ENET>,
> > -                                        <&clks
> IMX6SX_CLK_ENET_AHB>,
> > +                                        <&clks IMX6SX_CLK_ENET>,
> 
> Yes, there is really no IMX6SX_CLK_ENET_AHB as per the Refernce Manual and
> it is the same we do on imx6qdl.dtsi:
> 
> Reviewed-by: Fabio Estevam <festevam@xxxxxxxxx>

Copy Andy, the ENET owner, to comment.

BTW, it's strange that I did not receive the original patch email.
Also can't grep from the open list.

Regards
Dong Aisheng