All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-05 10:38 刘凯
  0 siblings, 0 replies; 12+ messages in thread
From: 刘凯 @ 2019-05-05 10:38 UTC (permalink / raw)
  To: Andy Duan
  Cc: Aisheng Dong, Fabio Estevam, Rob Herring, Mark Rutland,
	Shawn Guo, Sascha Hauer, Sascha Hauer, dl-linux-imx,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE 
	<linux-arm-kernel@lists.infradead.org>,
	linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 8493 bytes --]

Hi Andy,



The solution you submitted is work, but I'm very confused that the patch you submitted on Apr 9 is all your own work, because I submitted this problem to the Bugzilla on Apr 2, and sent an informal patch to the maintainer and list in Apr 8,  then Aisheng Dong replied me to asked me to send the patch in formal process and copy the patch and email to you, you just submitted this problem in the next day but didn't mention anything about me, even though I told Aisheng Dong I'll resend this patch in the formal process, This is too coincidental, isn't it?



In Apr 10, I resent the patch to the maintainer and list in the formal process(I'm sure linux-imx@nxp.com<mailto:linux-imx@nxp.com> was in the recipient), but Aisheng Dong said that he didn't receive this email, that's a coincidence again. This time Fabio replied me and told me how to write the commit log, and review my patch again and again(thanks to Fabio for his patient) , finally I have rewrote the commit log and resent the patch again in Apr 25, then you told me this problem was fixed and nack my patch.



I think that if you also find the problem in the same time, you can tell me when you receive the copy of the email in the first time, but you didn't, if you are not satisfied with the solution I was submitted, you can discuss with me, at least tell me, but you didn't, you just submitted this problem directly, your behavior makes me think that you have stolen my work, I spent two weeks analyzing this problem and another two weeks writing a formal patch, I'm very angry now, can you give me a reasonable explanation?



Ps: the evidence of Andy receive the copy of patch is in the attachments maybe it contains some Chinese words, but I think this has no effect on reading .



Kay Liu



-----Original Message-----

From: Andy Duan [mailto:fugang.duan@nxp.com]

Sent: Sunday, May 05, 2019 4:16 PM

To: Aisheng Dong <aisheng.dong@nxp.com<mailto:aisheng.dong@nxp.com>>; Fabio Estevam <festevam@gmail.com<mailto:festevam@gmail.com>>; 刘凯 <liuk@cetca.net.cn<mailto:liuk@cetca.net.cn>>

Cc: Rob Herring <robh+dt@kernel.org<mailto:robh+dt@kernel.org>>; Mark Rutland <mark.rutland@arm.com<mailto:mark.rutland@arm.com>>; Shawn Guo <shawnguo@kernel.org<mailto:shawnguo@kernel.org>>; Sascha Hauer <s.hauer@pengutronix.de<mailto:s.hauer@pengutronix.de>>; Sascha Hauer <kernel@pengutronix.de<mailto:kernel@pengutronix.de>>; dl-linux-imx <linux-imx@nxp.com<mailto:linux-imx@nxp.com>>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org<mailto:devicetree@vger.kernel.org>>; moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.infradead.org>>; linux-kernel <linux-kernel@vger.kernel.org<mailto:linux-kernel@vger.kernel.org>>

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



From: Aisheng Dong Sent: Sunday, May 5, 2019 4:03 PM

> > From: Fabio Estevam [mailto:festevam@gmail.com]

> > Sent: Saturday, May 4, 2019 7:04 PM

> >

> > Hi Kay-Liu,

> >

> > On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn<mailto:liuk@cetca.net.cn>> wrote:

> > >

> > > From: Kay-Liu <liuk@cetca.net.cn<mailto:liuk@cetca.net.cn>>

> > >

> > > 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@cetca.net.cn<mailto:liuk@cetca.net.cn>>

> > > ---

> > > 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@gmail.com<mailto:festevam@gmail.com>>



Nack the patch !



Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:

         IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB



IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s.

(Please check RM Table 18-3. System Clocks, Gating, and Override)



Secondly,  for your issue you caught, which was fixed by patch:

commit d7c3a206e6338e4ccdf030719dec028e26a521d5

Author: Andy Duan <fugang.duan@nxp.com<mailto:fugang.duan@nxp.com>>

Date:   Tue Apr 9 03:40:56 2019 +0000



    net: fec: manage ahb clock in runtime pm



    Some SOC like i.MX6SX clock have some limits:

    - ahb clock should be disabled before ipg.

    - ahb and ipg clocks are required for MAC MII bus.

    So, move the ahb clock to runtime management together with

    ipg clock.



    Signed-off-by: Fugang Duan <fugang.duan@nxp.com<mailto:fugang.duan@nxp.com>>

    Signed-off-by: David S. Miller <davem@davemloft.net<mailto:davem@davemloft.net>>





So, please don't remove ahb clock.



Andy

>

> 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


[-- Attachment #1.2: Type: text/html, Size: 22214 bytes --]

[-- Attachment #2: Aisheng Dong send a copy to Andy Duan.bmp --]
[-- Type: image/bmp, Size: 3189764 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
  2019-05-05  8:15       ` Andy Duan
  (?)
@ 2019-05-07 23:09         ` Fabio Estevam
  -1 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-05-07 23:09 UTC (permalink / raw)
  To: Andy Duan
  Cc: Aisheng Dong, Kay-Liu, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Sascha Hauer, dl-linux-imx,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Andy,

On Sun, May 5, 2019 at 5:15 AM Andy Duan <fugang.duan@nxp.com> wrote:

> Nack the patch !
>
> Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:
>         IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB
>
> IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s.
> (Please check RM Table 18-3. System Clocks, Gating, and Override)

Ok, but could you please show us where in the Reference Manual the
IMX6SX_CLK_ENET_AHB is mentioned?

I don't see ENET_AHB in imx6qdl Reference Manual either and we don't
have a ENET_AHB the clk-imx6q driver and nor in the devicetree,

> Secondly,  for your issue you caught, which was fixed by patch:
> commit d7c3a206e6338e4ccdf030719dec028e26a521d5
> Author: Andy Duan <fugang.duan@nxp.com>
> Date:   Tue Apr 9 03:40:56 2019 +0000
>
>     net: fec: manage ahb clock in runtime pm

Would this also fix the case where power management support is disabled?

If I understand correctly the explanation from Kay-Liu he would still
see a hang in the case when PM is disabled.

Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-07 23:09         ` Fabio Estevam
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-05-07 23:09 UTC (permalink / raw)
  To: Andy Duan
  Cc: Aisheng Dong, Kay-Liu, Rob Herring, Mark Rutland, Shawn Guo,
	Sascha Hauer, Sascha Hauer, dl-linux-imx,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Andy,

On Sun, May 5, 2019 at 5:15 AM Andy Duan <fugang.duan@nxp.com> wrote:

> Nack the patch !
>
> Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:
>         IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB
>
> IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s.
> (Please check RM Table 18-3. System Clocks, Gating, and Override)

Ok, but could you please show us where in the Reference Manual the
IMX6SX_CLK_ENET_AHB is mentioned?

I don't see ENET_AHB in imx6qdl Reference Manual either and we don't
have a ENET_AHB the clk-imx6q driver and nor in the devicetree,

> Secondly,  for your issue you caught, which was fixed by patch:
> commit d7c3a206e6338e4ccdf030719dec028e26a521d5
> Author: Andy Duan <fugang.duan@nxp.com>
> Date:   Tue Apr 9 03:40:56 2019 +0000
>
>     net: fec: manage ahb clock in runtime pm

Would this also fix the case where power management support is disabled?

If I understand correctly the explanation from Kay-Liu he would still
see a hang in the case when PM is disabled.

Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-07 23:09         ` Fabio Estevam
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-05-07 23:09 UTC (permalink / raw)
  To: Andy Duan
  Cc: Aisheng Dong, Mark Rutland, Kay-Liu,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Sascha Hauer, linux-kernel, Rob Herring, dl-linux-imx,
	Sascha Hauer, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

Hi Andy,

On Sun, May 5, 2019 at 5:15 AM Andy Duan <fugang.duan@nxp.com> wrote:

> Nack the patch !
>
> Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:
>         IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB
>
> IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s.
> (Please check RM Table 18-3. System Clocks, Gating, and Override)

Ok, but could you please show us where in the Reference Manual the
IMX6SX_CLK_ENET_AHB is mentioned?

I don't see ENET_AHB in imx6qdl Reference Manual either and we don't
have a ENET_AHB the clk-imx6q driver and nor in the devicetree,

> Secondly,  for your issue you caught, which was fixed by patch:
> commit d7c3a206e6338e4ccdf030719dec028e26a521d5
> Author: Andy Duan <fugang.duan@nxp.com>
> Date:   Tue Apr 9 03:40:56 2019 +0000
>
>     net: fec: manage ahb clock in runtime pm

Would this also fix the case where power management support is disabled?

If I understand correctly the explanation from Kay-Liu he would still
see a hang in the case when PM is disabled.

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
  2019-05-05  8:03     ` Aisheng Dong
  (?)
@ 2019-05-05  8:15       ` Andy Duan
  -1 siblings, 0 replies; 12+ messages in thread
From: Andy Duan @ 2019-05-05  8:15 UTC (permalink / raw)
  To: Aisheng Dong, Fabio Estevam, Kay-Liu
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	dl-linux-imx,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

From: Aisheng Dong Sent: Sunday, May 5, 2019 4:03 PM
> > From: Fabio Estevam [mailto:festevam@gmail.com]
> > Sent: Saturday, May 4, 2019 7:04 PM
> >
> > Hi Kay-Liu,
> >
> > On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
> > >
> > > From: Kay-Liu <liuk@cetca.net.cn>
> > >
> > > 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@cetca.net.cn>
> > > ---
> > > 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@gmail.com>

Nack the patch !

Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:
	IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB

IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s. 
(Please check RM Table 18-3. System Clocks, Gating, and Override)

Secondly,  for your issue you caught, which was fixed by patch:
commit d7c3a206e6338e4ccdf030719dec028e26a521d5
Author: Andy Duan <fugang.duan@nxp.com>
Date:   Tue Apr 9 03:40:56 2019 +0000

    net: fec: manage ahb clock in runtime pm

    Some SOC like i.MX6SX clock have some limits:
    - ahb clock should be disabled before ipg.
    - ahb and ipg clocks are required for MAC MII bus.
    So, move the ahb clock to runtime management together with
    ipg clock.

    Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
    Signed-off-by: David S. Miller <davem@davemloft.net> 


So, please don't remove ahb clock.

Andy
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-05  8:15       ` Andy Duan
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Duan @ 2019-05-05  8:15 UTC (permalink / raw)
  To: Aisheng Dong, Fabio Estevam, Kay-Liu
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	dl-linux-imx,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

From: Aisheng Dong Sent: Sunday, May 5, 2019 4:03 PM
> > From: Fabio Estevam [mailto:festevam@gmail.com]
> > Sent: Saturday, May 4, 2019 7:04 PM
> >
> > Hi Kay-Liu,
> >
> > On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
> > >
> > > From: Kay-Liu <liuk@cetca.net.cn>
> > >
> > > 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@cetca.net.cn>
> > > ---
> > > 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@gmail.com>

Nack the patch !

Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:
	IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB

IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s. 
(Please check RM Table 18-3. System Clocks, Gating, and Override)

Secondly,  for your issue you caught, which was fixed by patch:
commit d7c3a206e6338e4ccdf030719dec028e26a521d5
Author: Andy Duan <fugang.duan@nxp.com>
Date:   Tue Apr 9 03:40:56 2019 +0000

    net: fec: manage ahb clock in runtime pm

    Some SOC like i.MX6SX clock have some limits:
    - ahb clock should be disabled before ipg.
    - ahb and ipg clocks are required for MAC MII bus.
    So, move the ahb clock to runtime management together with
    ipg clock.

    Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
    Signed-off-by: David S. Miller <davem@davemloft.net> 


So, please don't remove ahb clock.

Andy
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-05  8:15       ` Andy Duan
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Duan @ 2019-05-05  8:15 UTC (permalink / raw)
  To: Aisheng Dong, Fabio Estevam, Kay-Liu
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Sascha Hauer, linux-kernel, Rob Herring, dl-linux-imx,
	Sascha Hauer, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

From: Aisheng Dong Sent: Sunday, May 5, 2019 4:03 PM
> > From: Fabio Estevam [mailto:festevam@gmail.com]
> > Sent: Saturday, May 4, 2019 7:04 PM
> >
> > Hi Kay-Liu,
> >
> > On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
> > >
> > > From: Kay-Liu <liuk@cetca.net.cn>
> > >
> > > 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@cetca.net.cn>
> > > ---
> > > 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@gmail.com>

Nack the patch !

Firstly, i.MX6SX has ENET AHB bus clock for MAC, and currently it is set 200Mhz like clock tree:
	IMX6SX_CLK_ENET_PODF 200Mhz -> IMX6SX_CLK_ENET_SEL -> IMX6SX_CLK_ENET_AHB

IMX6SX_CLK_ENET the clock is IPG clock for ENET IP ipg_clk_mac0_s/ipg_clk_s. 
(Please check RM Table 18-3. System Clocks, Gating, and Override)

Secondly,  for your issue you caught, which was fixed by patch:
commit d7c3a206e6338e4ccdf030719dec028e26a521d5
Author: Andy Duan <fugang.duan@nxp.com>
Date:   Tue Apr 9 03:40:56 2019 +0000

    net: fec: manage ahb clock in runtime pm

    Some SOC like i.MX6SX clock have some limits:
    - ahb clock should be disabled before ipg.
    - ahb and ipg clocks are required for MAC MII bus.
    So, move the ahb clock to runtime management together with
    ipg clock.

    Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
    Signed-off-by: David S. Miller <davem@davemloft.net> 


So, please don't remove ahb clock.

Andy
> 
> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
  2019-05-04 11:03   ` Fabio Estevam
@ 2019-05-05  8:03     ` Aisheng Dong
  -1 siblings, 0 replies; 12+ messages in thread
From: Aisheng Dong @ 2019-05-05  8:03 UTC (permalink / raw)
  To: Fabio Estevam, Kay-Liu, Andy Duan
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	dl-linux-imx,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Saturday, May 4, 2019 7:04 PM
> 
> Hi Kay-Liu,
> 
> On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
> >
> > From: Kay-Liu <liuk@cetca.net.cn>
> >
> > 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@cetca.net.cn>
> > ---
> > 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@gmail.com>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-05  8:03     ` Aisheng Dong
  0 siblings, 0 replies; 12+ messages in thread
From: Aisheng Dong @ 2019-05-05  8:03 UTC (permalink / raw)
  To: Fabio Estevam, Kay-Liu, Andy Duan
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Sascha Hauer, linux-kernel, Rob Herring, dl-linux-imx,
	Sascha Hauer, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Saturday, May 4, 2019 7:04 PM
> 
> Hi Kay-Liu,
> 
> On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
> >
> > From: Kay-Liu <liuk@cetca.net.cn>
> >
> > 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@cetca.net.cn>
> > ---
> > 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@gmail.com>

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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
       [not found] <1556190530-19541-1-git-send-email-liuk@cetca.net.cn>
  2019-05-04 11:03   ` Fabio Estevam
@ 2019-05-04 11:03   ` Fabio Estevam
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-05-04 11:03 UTC (permalink / raw)
  To: Kay-Liu
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	NXP Linux Team,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Kay-Liu,

On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
>
> From: Kay-Liu <liuk@cetca.net.cn>
>
> 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@cetca.net.cn>
> ---
> 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@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-04 11:03   ` Fabio Estevam
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-05-04 11:03 UTC (permalink / raw)
  To: Kay-Liu
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	NXP Linux Team,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Kay-Liu,

On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
>
> From: Kay-Liu <liuk@cetca.net.cn>
>
> 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@cetca.net.cn>
> ---
> 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@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock
@ 2019-05-04 11:03   ` Fabio Estevam
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2019-05-04 11:03 UTC (permalink / raw)
  To: Kay-Liu
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Sascha Hauer, linux-kernel, Rob Herring, NXP Linux Team,
	Sascha Hauer, Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

Hi Kay-Liu,

On Thu, Apr 25, 2019 at 8:09 AM <liuk@cetca.net.cn> wrote:
>
> From: Kay-Liu <liuk@cetca.net.cn>
>
> 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@cetca.net.cn>
> ---
> 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@gmail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-05-07 23:10 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-05 10:38 [PATCHv2 1/2] ARM: dts: imx6sx: Use MX6SX_CLK_ENET for fec 'ahb' clock 刘凯
     [not found] <1556190530-19541-1-git-send-email-liuk@cetca.net.cn>
2019-05-04 11:03 ` Fabio Estevam
2019-05-04 11:03   ` Fabio Estevam
2019-05-04 11:03   ` Fabio Estevam
2019-05-05  8:03   ` Aisheng Dong
2019-05-05  8:03     ` Aisheng Dong
2019-05-05  8:15     ` Andy Duan
2019-05-05  8:15       ` Andy Duan
2019-05-05  8:15       ` Andy Duan
2019-05-07 23:09       ` Fabio Estevam
2019-05-07 23:09         ` Fabio Estevam
2019-05-07 23:09         ` Fabio Estevam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.