| Message ID | 3ceda169-de1b-2c1f-9ee8-bc8fdb547433@i2se.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | imx6ul: Recent enet refclock changes breaks custom i.mx6ull board | expand |
Hi Stefan, On Sun, Mar 05, 2023 at 11:16:17PM +0100, Stefan Wahren wrote: > Hi, > > we planned to submit our custom i.MX6ULL board [1] to mainline after release > of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet > phy: > > [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) > > ... > > [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset > failed: -110 > [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy > > I narrow down the PHY issue to this first bad commit: > > 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") > > The clock issues seems to be cause by the following commit. If i revert > 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. It looks like in your kernel version are some missing patches. Can you please rebase your patches on top of this branch: https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/log/?h=for-next and please rename IMX6UL_CLK_ENET_REF to IMX6UL_CLK_ENET1_REF_SEL in your dtsi. Regards, Oleksij
Hi Oleksij, Am 06.03.23 um 06:25 schrieb Oleksij Rempel: > Hi Stefan, > > On Sun, Mar 05, 2023 at 11:16:17PM +0100, Stefan Wahren wrote: >> Hi, >> >> we planned to submit our custom i.MX6ULL board [1] to mainline after release >> of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet >> phy: >> >> [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) >> >> ... >> >> [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset >> failed: -110 >> [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy >> >> I narrow down the PHY issue to this first bad commit: >> >> 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") >> >> The clock issues seems to be cause by the following commit. If i revert >> 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. > It looks like in your kernel version are some missing patches. Can you please > rebase your patches on top of this branch: > https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/log/?h=for-next thanks for your fast reply. But i rebased my patches against Linux v6.3-rc1 since this was released yesterday and should contain all patches from Shawn. I also changed the clockref in my DTSI file: https://github.com/chargebyte/linux/commits/v6.3-tarragon-v3 Now the PHY issue disappeared and ethernet is working, but the imx:clk-gpr-mux: failed to get parent (-EINVAL) is still there. > and please rename IMX6UL_CLK_ENET_REF to IMX6UL_CLK_ENET1_REF_SEL in > your dtsi. Yes, this seems to be the issue in my case. Does this mean a Linux 6.3 kernel doesn't work with a i.MX6ULL Linux 6.2 devicetree? So there is no fallback? What about these other dtsi in Linux 6.3rc-1? $ grep IMX6UL_CLK_ENET_REF * imx6ul-14x14-evk.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; .. imx6ul-kontron-bl-common.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; imx6ul-kontron-sl-common.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; imx6ull-dhcom-picoitx.dts: clocks = <&clks IMX6UL_CLK_ENET_REF>; imx6ull-dhcom-som.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; imx6ull-jozacp.dts: clocks = <&clks IMX6UL_CLK_ENET_REF>; imx6ull-myir-mys-6ulx.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; imx6ul-phytec-phycore-som.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; mba6ulx.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; > > Regards, > Oleksij
On Mon, Mar 06, 2023 at 10:13:57AM +0100, Stefan Wahren wrote: > Hi Oleksij, > > Am 06.03.23 um 06:25 schrieb Oleksij Rempel: > > Hi Stefan, > > > > On Sun, Mar 05, 2023 at 11:16:17PM +0100, Stefan Wahren wrote: > > > Hi, > > > > > > we planned to submit our custom i.MX6ULL board [1] to mainline after release > > > of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet > > > phy: > > > > > > [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) > > > > > > ... > > > > > > [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset > > > failed: -110 > > > [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy > > > > > > I narrow down the PHY issue to this first bad commit: > > > > > > 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") > > > > > > The clock issues seems to be cause by the following commit. If i revert > > > 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. > > It looks like in your kernel version are some missing patches. Can you please > > rebase your patches on top of this branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/log/?h=for-next > > thanks for your fast reply. But i rebased my patches against Linux v6.3-rc1 > since this was released yesterday and should contain all patches from Shawn. No, it is not. Related DTS changes are not included in to v6.3-rc1. > I also changed the clockref in my DTSI file: > > https://github.com/chargebyte/linux/commits/v6.3-tarragon-v3 > > Now the PHY issue disappeared and ethernet is working, but the > > imx:clk-gpr-mux: failed to get parent (-EINVAL) I need to take a look at it. It should not be critical. > > is still there. > > > and please rename IMX6UL_CLK_ENET_REF to IMX6UL_CLK_ENET1_REF_SEL in > > your dtsi. > > Yes, this seems to be the issue in my case. > > Does this mean a Linux 6.3 kernel doesn't work with a i.MX6ULL Linux 6.2 > devicetree? If I see it correctly. Since you do not have patch [1] related clock is not enabled by the fec controller. Since this PHY is not addressable without running rmii clock, the PHY can't be probed. > So there is no fallback? With [1] it should not be needed. > > What about these other dtsi in Linux 6.3rc-1? > > $ grep IMX6UL_CLK_ENET_REF * > imx6ul-14x14-evk.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; > .. > imx6ul-kontron-bl-common.dtsi: clocks = <&clks > IMX6UL_CLK_ENET_REF>; > imx6ul-kontron-sl-common.dtsi: clocks = <&clks > IMX6UL_CLK_ENET_REF>; > imx6ull-dhcom-picoitx.dts: clocks = <&clks IMX6UL_CLK_ENET_REF>; > imx6ull-dhcom-som.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; > imx6ull-jozacp.dts: clocks = <&clks IMX6UL_CLK_ENET_REF>; > imx6ull-myir-mys-6ulx.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; > imx6ul-phytec-phycore-som.dtsi: clocks = <&clks > IMX6UL_CLK_ENET_REF>; > mba6ulx.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; It is nice to convert all of them to proper clock. But all of them are expected to work with [1]. Can you please confirm it? Revert yourdtsi back to IMX6UL_CLK_ENET_REF and include [1]? [1] https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/?h=for-next&id=8940c105273fcde00a60023f68f8a5b75e1df0cc Regards, Oleksij
Hi Oleksij, Am 06.03.23 um 10:47 schrieb Oleksij Rempel: > On Mon, Mar 06, 2023 at 10:13:57AM +0100, Stefan Wahren wrote: >> Hi Oleksij, >> >> Am 06.03.23 um 06:25 schrieb Oleksij Rempel: >>> Hi Stefan, >>> >>> On Sun, Mar 05, 2023 at 11:16:17PM +0100, Stefan Wahren wrote: >>>> Hi, >>>> >>>> we planned to submit our custom i.MX6ULL board [1] to mainline after release >>>> of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet >>>> phy: >>>> >>>> [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) >>>> >>>> ... >>>> >>>> [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset >>>> failed: -110 >>>> [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy >>>> >>>> I narrow down the PHY issue to this first bad commit: >>>> >>>> 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") >>>> >>>> The clock issues seems to be cause by the following commit. If i revert >>>> 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. >>> It looks like in your kernel version are some missing patches. Can you please >>> rebase your patches on top of this branch: >>> https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/log/?h=for-next >> thanks for your fast reply. But i rebased my patches against Linux v6.3-rc1 >> since this was released yesterday and should contain all patches from Shawn. > No, it is not. Related DTS changes are not included in to v6.3-rc1. Sorry, i didn't noticed that Shawn already rebased his for-next changes on top of v6.3-rc1. So, the problem is that your clk changes has been applied for 6.3, but the necessary arm changes will land in 6.4? :-( > >> I also changed the clockref in my DTSI file: >> >> https://github.com/chargebyte/linux/commits/v6.3-tarragon-v3 >> >> Now the PHY issue disappeared and ethernet is working, but the >> >> imx:clk-gpr-mux: failed to get parent (-EINVAL) > I need to take a look at it. It should not be critical. I prepared a patch [1] to improve the debugging here: [ 0.000000] Entry 262144 != val 0 [ 0.000000] Entry 16384 != val 0 [ 0.000000] imx:clk-gpr-mux: val 0, num_parents 2 [ 0.000000] imx:clk-gpr-mux: failed to get parent of enet2_ref_sel (-EINVAL) It seems that val 0 is unexpected for the driver. Maybe it's worth to mention that we use an older U-Boot [2]. But Linux should make any assumptions here. > >> is still there. >> >>> and please rename IMX6UL_CLK_ENET_REF to IMX6UL_CLK_ENET1_REF_SEL in >>> your dtsi. >> Yes, this seems to be the issue in my case. >> >> Does this mean a Linux 6.3 kernel doesn't work with a i.MX6ULL Linux 6.2 >> devicetree? > If I see it correctly. Since you do not have patch [1] related clock is not > enabled by the fec controller. Since this PHY is not addressable without > running rmii clock, the PHY can't be probed. > >> So there is no fallback? > With [1] it should not be needed. Maybe this patch comes too late (Linux 6.4) for some boards. > >> What about these other dtsi in Linux 6.3rc-1? >> >> $ grep IMX6UL_CLK_ENET_REF * >> imx6ul-14x14-evk.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; >> .. >> imx6ul-kontron-bl-common.dtsi: clocks = <&clks >> IMX6UL_CLK_ENET_REF>; >> imx6ul-kontron-sl-common.dtsi: clocks = <&clks >> IMX6UL_CLK_ENET_REF>; >> imx6ull-dhcom-picoitx.dts: clocks = <&clks IMX6UL_CLK_ENET_REF>; >> imx6ull-dhcom-som.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; >> imx6ull-jozacp.dts: clocks = <&clks IMX6UL_CLK_ENET_REF>; >> imx6ull-myir-mys-6ulx.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; >> imx6ul-phytec-phycore-som.dtsi: clocks = <&clks >> IMX6UL_CLK_ENET_REF>; >> mba6ulx.dtsi: clocks = <&clks IMX6UL_CLK_ENET_REF>; > It is nice to convert all of them to proper clock. But all of them are > expected to work with [1]. > > Can you please confirm it? Revert yourdtsi back to IMX6UL_CLK_ENET_REF > and include [1]? I rebased all changes on top of Shawn's branch and reverted to IMX6UL_CLK_ENET_REF [3]. So yes, i confirm that Ethernet works in this case. Best regards [1] - https://github.com/chargebyte/linux/commit/74a883d3ca4960a7d178d4a184daf9856600ca14 [2] - https://github.com/chargebyte/U-Boot/tree/v2020.04-in-tech [3] - https://github.com/chargebyte/linux/tree/v6.4-shawnguo-tarragon > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/commit/?h=for-next&id=8940c105273fcde00a60023f68f8a5b75e1df0cc > > Regards, > Oleksij
On Mon, Mar 06, 2023 at 02:33:24PM +0100, Stefan Wahren wrote: > Hi Oleksij, > > Am 06.03.23 um 10:47 schrieb Oleksij Rempel: > > On Mon, Mar 06, 2023 at 10:13:57AM +0100, Stefan Wahren wrote: > > > Hi Oleksij, > > > > > > Am 06.03.23 um 06:25 schrieb Oleksij Rempel: > > > > Hi Stefan, > > > > > > > > On Sun, Mar 05, 2023 at 11:16:17PM +0100, Stefan Wahren wrote: > > > > > Hi, > > > > > > > > > > we planned to submit our custom i.MX6ULL board [1] to mainline after release > > > > > of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet > > > > > phy: > > > > > > > > > > [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) > > > > > > > > > > ... > > > > > > > > > > [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset > > > > > failed: -110 > > > > > [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy > > > > > > > > > > I narrow down the PHY issue to this first bad commit: > > > > > > > > > > 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") > > > > > > > > > > The clock issues seems to be cause by the following commit. If i revert > > > > > 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. > > > > It looks like in your kernel version are some missing patches. Can you please > > > > rebase your patches on top of this branch: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/log/?h=for-next > > > thanks for your fast reply. But i rebased my patches against Linux v6.3-rc1 > > > since this was released yesterday and should contain all patches from Shawn. > > No, it is not. Related DTS changes are not included in to v6.3-rc1. > > Sorry, i didn't noticed that Shawn already rebased his for-next changes on > top of v6.3-rc1. > > So, the problem is that your clk changes has been applied for 6.3, but the > necessary arm changes will land in 6.4? :-( I hope it will go as fixes to 6.3-rcX. Shawn? > > > I also changed the clockref in my DTSI file: > > > > > > https://github.com/chargebyte/linux/commits/v6.3-tarragon-v3 > > > > > > Now the PHY issue disappeared and ethernet is working, but the > > > > > > imx:clk-gpr-mux: failed to get parent (-EINVAL) > > I need to take a look at it. It should not be critical. > > I prepared a patch [1] to improve the debugging here: > > [ 0.000000] Entry 262144 != val 0 > [ 0.000000] Entry 16384 != val 0 > [ 0.000000] imx:clk-gpr-mux: val 0, num_parents 2 > [ 0.000000] imx:clk-gpr-mux: failed to get parent of enet2_ref_sel > (-EINVAL) > > It seems that val 0 is unexpected for the driver. Maybe it's worth to > mention that we use an older U-Boot [2]. But Linux should make any > assumptions here. There are two configuration bits per Ethernet interface: - BIT(17) ENET1_TX_CLK_DIR - BIT(13) ENET1_CLK_SEL With this bits we have following variants: 1. internal clock source with output on ENET1_TX_CLK 2. internal clock source without output on ENET1_TX_CLK. Are there any use cases need to support this mode? 3. external clock source without output on ENET1_TX_CLK 4. external clock source with output on ENET1_TX_CLK, well ENET1_TX_CLK is input it can't be out put on this case. Current kernel supports modes 1 and 3. For mode 2 I do not have a use case and mode 4 make no sense. In your case, the boot loader configures clocks to mode 2 which is not correct for this HW. It should be mode 1. Probably, the way to go is do register dummy parents for not supported modes. It would silent the kernel. Other ideas? > > Can you please confirm it? Revert yourdtsi back to IMX6UL_CLK_ENET_REF > > and include [1]? > > I rebased all changes on top of Shawn's branch and reverted to > IMX6UL_CLK_ENET_REF [3]. So yes, i confirm that Ethernet works in this case. Thx! So, there should be no regressions if this patch will to as fix for 6.3-rcX. Except of kernel warning wrong parent configuration. Regards, Oleksij
Hi Oleksij, Am 06.03.23 um 15:02 schrieb Oleksij Rempel: > On Mon, Mar 06, 2023 at 02:33:24PM +0100, Stefan Wahren wrote: >> Hi Oleksij, >> >> Am 06.03.23 um 10:47 schrieb Oleksij Rempel: >>> On Mon, Mar 06, 2023 at 10:13:57AM +0100, Stefan Wahren wrote: >>>> Hi Oleksij, >>>> >>>> Am 06.03.23 um 06:25 schrieb Oleksij Rempel: >>>>> Hi Stefan, >>>>> >>>>> On Sun, Mar 05, 2023 at 11:16:17PM +0100, Stefan Wahren wrote: >>>>>> Hi, >>>>>> >>>>>> we planned to submit our custom i.MX6ULL board [1] to mainline after release >>>>>> of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet >>>>>> phy: >>>>>> >>>>>> [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) >>>>>> >>>>>> ... >>>>>> >>>>>> [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset >>>>>> failed: -110 >>>>>> [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy >>>>>> >>>>>> I narrow down the PHY issue to this first bad commit: >>>>>> >>>>>> 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") >>>>>> >>>>>> The clock issues seems to be cause by the following commit. If i revert >>>>>> 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. >>>>> It looks like in your kernel version are some missing patches. Can you please >>>>> rebase your patches on top of this branch: >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git/log/?h=for-next >>>> thanks for your fast reply. But i rebased my patches against Linux v6.3-rc1 >>>> since this was released yesterday and should contain all patches from Shawn. >>> No, it is not. Related DTS changes are not included in to v6.3-rc1. >> Sorry, i didn't noticed that Shawn already rebased his for-next changes on >> top of v6.3-rc1. >> >> So, the problem is that your clk changes has been applied for 6.3, but the >> necessary arm changes will land in 6.4? :-( > I hope it will go as fixes to 6.3-rcX. Shawn? > >>>> I also changed the clockref in my DTSI file: >>>> >>>> https://github.com/chargebyte/linux/commits/v6.3-tarragon-v3 >>>> >>>> Now the PHY issue disappeared and ethernet is working, but the >>>> >>>> imx:clk-gpr-mux: failed to get parent (-EINVAL) >>> I need to take a look at it. It should not be critical. >> I prepared a patch [1] to improve the debugging here: >> >> [ 0.000000] Entry 262144 != val 0 >> [ 0.000000] Entry 16384 != val 0 >> [ 0.000000] imx:clk-gpr-mux: val 0, num_parents 2 >> [ 0.000000] imx:clk-gpr-mux: failed to get parent of enet2_ref_sel >> (-EINVAL) >> >> It seems that val 0 is unexpected for the driver. Maybe it's worth to >> mention that we use an older U-Boot [2]. But Linux should make any >> assumptions here. > There are two configuration bits per Ethernet interface: > - BIT(17) ENET1_TX_CLK_DIR > - BIT(13) ENET1_CLK_SEL Did you noticed that the error is caused for enet2_ref_sel? On our board variants master/slave/slaveXT only ENET1 is used, so ENET2 is kept to the defaults (ENET2_TX_CLK_DIR = 0, ENET2_CLK_SEL = 0) and the bootloader won't touch those bits. > With this bits we have following variants: > 1. internal clock source with output on ENET1_TX_CLK > 2. internal clock source without output on ENET1_TX_CLK. Are there any > use cases need to support this mode? After reading the reference manual, this mode refers to ENET1_TX_CLK_DIR = 0, ENET1_CLK_SEL = 0. Is my understanding correct? > 3. external clock source without output on ENET1_TX_CLK > 4. external clock source with output on ENET1_TX_CLK, well ENET1_TX_CLK > is input it can't be out put on this case. > > Current kernel supports modes 1 and 3. For mode 2 I do not have a use > case and mode 4 make no sense. > > In your case, the boot loader configures clocks to mode 2 which is not > correct for this HW. It should be mode 1. As written above the bootloader doesn't touch this. It's the reset default according to the reference manual. So i consider mode 2 as disabled clock, which is the right mode for boards without using this particular Ethernet interface. For EMC reasons we don't want to enable ENET1 and ENET2 clock output unconditionally. > Probably, the way to go is do register dummy parents for not supported > modes. It would silent the kernel. Other ideas? Sorry, i don't have no idea how to properly achieve this. Best regards > >>> Can you please confirm it? Revert yourdtsi back to IMX6UL_CLK_ENET_REF >>> and include [1]? >> I rebased all changes on top of Shawn's branch and reverted to >> IMX6UL_CLK_ENET_REF [3]. So yes, i confirm that Ethernet works in this case. > Thx! So, there should be no regressions if this patch will to as fix for > 6.3-rcX. Except of kernel warning wrong parent configuration. > > Regards, > Oleksij
Hi Stefan, On Mon, Mar 06, 2023 at 04:50:18PM +0100, Stefan Wahren wrote: > Did you noticed that the error is caused for enet2_ref_sel? > > On our board variants master/slave/slaveXT only ENET1 is used, so ENET2 is > kept to the defaults (ENET2_TX_CLK_DIR = 0, ENET2_CLK_SEL = 0) and the > bootloader won't touch those bits. Ok, i see. It makes sense. > > With this bits we have following variants: > > 1. internal clock source with output on ENET1_TX_CLK > > 2. internal clock source without output on ENET1_TX_CLK. Are there any > > use cases need to support this mode? > After reading the reference manual, this mode refers to ENET1_TX_CLK_DIR = > 0, ENET1_CLK_SEL = 0. Is my understanding correct? > > 3. external clock source without output on ENET1_TX_CLK > > 4. external clock source with output on ENET1_TX_CLK, well ENET1_TX_CLK > > is input it can't be out put on this case. > > > > Current kernel supports modes 1 and 3. For mode 2 I do not have a use > > case and mode 4 make no sense. > > > > In your case, the boot loader configures clocks to mode 2 which is not > > correct for this HW. It should be mode 1. > As written above the bootloader doesn't touch this. It's the reset default > according to the reference manual. So i consider mode 2 as disabled clock, > which is the right mode for boards without using this particular Ethernet > interface. For EMC reasons we don't want to enable ENET1 and ENET2 clock > output unconditionally. > > Probably, the way to go is do register dummy parents for not supported > > modes. It would silent the kernel. Other ideas? > > Sorry, i don't have no idea how to properly achieve this. can you please test this patch: diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c index 2836adb817b7..e3696a88b5a3 100644 --- a/drivers/clk/imx/clk-imx6ul.c +++ b/drivers/clk/imx/clk-imx6ul.c @@ -95,14 +95,16 @@ static const struct clk_div_table video_div_table[] = { { } }; -static const char * enet1_ref_sels[] = { "enet1_ref_125m", "enet1_ref_pad", }; +static const char * enet1_ref_sels[] = { "enet1_ref_125m", "enet1_ref_pad", "dummy", "dummy"}; static const u32 enet1_ref_sels_table[] = { IMX6UL_GPR1_ENET1_TX_CLK_DIR, - IMX6UL_GPR1_ENET1_CLK_SEL }; + IMX6UL_GPR1_ENET1_CLK_SEL, 0, + IMX6UL_GPR1_ENET1_TX_CLK_DIR | IMX6UL_GPR1_ENET1_CLK_SEL }; static const u32 enet1_ref_sels_table_mask = IMX6UL_GPR1_ENET1_TX_CLK_DIR | IMX6UL_GPR1_ENET1_CLK_SEL; -static const char * enet2_ref_sels[] = { "enet2_ref_125m", "enet2_ref_pad", }; +static const char * enet2_ref_sels[] = { "enet2_ref_125m", "enet2_ref_pad", "dummy", "dummy"}; static const u32 enet2_ref_sels_table[] = { IMX6UL_GPR1_ENET2_TX_CLK_DIR, - IMX6UL_GPR1_ENET2_CLK_SEL }; + IMX6UL_GPR1_ENET2_CLK_SEL, 0, + IMX6UL_GPR1_ENET2_TX_CLK_DIR | IMX6UL_GPR1_ENET2_CLK_SEL }; static const u32 enet2_ref_sels_table_mask = IMX6UL_GPR1_ENET2_TX_CLK_DIR | IMX6UL_GPR1_ENET2_CLK_SEL; Regards, Oleksij
Hi Oleksij, Am 07.03.23 um 07:06 schrieb Oleksij Rempel: > Hi Stefan, > > On Mon, Mar 06, 2023 at 04:50:18PM +0100, Stefan Wahren wrote: >> Did you noticed that the error is caused for enet2_ref_sel? >> >> On our board variants master/slave/slaveXT only ENET1 is used, so ENET2 is >> kept to the defaults (ENET2_TX_CLK_DIR = 0, ENET2_CLK_SEL = 0) and the >> bootloader won't touch those bits. > Ok, i see. It makes sense. > >>> With this bits we have following variants: >>> 1. internal clock source with output on ENET1_TX_CLK >>> 2. internal clock source without output on ENET1_TX_CLK. Are there any >>> use cases need to support this mode? >> After reading the reference manual, this mode refers to ENET1_TX_CLK_DIR = >> 0, ENET1_CLK_SEL = 0. Is my understanding correct? >>> 3. external clock source without output on ENET1_TX_CLK >>> 4. external clock source with output on ENET1_TX_CLK, well ENET1_TX_CLK >>> is input it can't be out put on this case. >>> >>> Current kernel supports modes 1 and 3. For mode 2 I do not have a use >>> case and mode 4 make no sense. >>> >>> In your case, the boot loader configures clocks to mode 2 which is not >>> correct for this HW. It should be mode 1. >> As written above the bootloader doesn't touch this. It's the reset default >> according to the reference manual. So i consider mode 2 as disabled clock, >> which is the right mode for boards without using this particular Ethernet >> interface. For EMC reasons we don't want to enable ENET1 and ENET2 clock >> output unconditionally. >>> Probably, the way to go is do register dummy parents for not supported >>> modes. It would silent the kernel. Other ideas? >> Sorry, i don't have no idea how to properly achieve this. > can you please test this patch: > > diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c > index 2836adb817b7..e3696a88b5a3 100644 > --- a/drivers/clk/imx/clk-imx6ul.c > +++ b/drivers/clk/imx/clk-imx6ul.c > @@ -95,14 +95,16 @@ static const struct clk_div_table video_div_table[] = { > { } > }; > > -static const char * enet1_ref_sels[] = { "enet1_ref_125m", "enet1_ref_pad", }; > +static const char * enet1_ref_sels[] = { "enet1_ref_125m", "enet1_ref_pad", "dummy", "dummy"}; > static const u32 enet1_ref_sels_table[] = { IMX6UL_GPR1_ENET1_TX_CLK_DIR, > - IMX6UL_GPR1_ENET1_CLK_SEL }; > + IMX6UL_GPR1_ENET1_CLK_SEL, 0, > + IMX6UL_GPR1_ENET1_TX_CLK_DIR | IMX6UL_GPR1_ENET1_CLK_SEL }; > static const u32 enet1_ref_sels_table_mask = IMX6UL_GPR1_ENET1_TX_CLK_DIR | > IMX6UL_GPR1_ENET1_CLK_SEL; > -static const char * enet2_ref_sels[] = { "enet2_ref_125m", "enet2_ref_pad", }; > +static const char * enet2_ref_sels[] = { "enet2_ref_125m", "enet2_ref_pad", "dummy", "dummy"}; > static const u32 enet2_ref_sels_table[] = { IMX6UL_GPR1_ENET2_TX_CLK_DIR, > - IMX6UL_GPR1_ENET2_CLK_SEL }; > + IMX6UL_GPR1_ENET2_CLK_SEL, 0, > + IMX6UL_GPR1_ENET2_TX_CLK_DIR | IMX6UL_GPR1_ENET2_CLK_SEL }; > static const u32 enet2_ref_sels_table_mask = IMX6UL_GPR1_ENET2_TX_CLK_DIR | > IMX6UL_GPR1_ENET2_CLK_SEL; i successful tested this patch on top of Shawn's for-next branch. The error message went away. Just 2 ideas for a proper patch: - short explaining comment in clk-imx6ul about the dummies - instead of "dummy" for both interfaces, i suggest something like "enet1_ref_dummy" which makes investigation at /sys/kernel/debug/clk/clk_summary easier Thanks Stefan > > Regards, > Oleksij
Am 08.03.23 um 16:11 schrieb Stefan Wahren: > Hi Oleksij, > > Am 07.03.23 um 07:06 schrieb Oleksij Rempel: >> Hi Stefan, >> >> On Mon, Mar 06, 2023 at 04:50:18PM +0100, Stefan Wahren wrote: >>> Did you noticed that the error is caused for enet2_ref_sel? >>> >>> On our board variants master/slave/slaveXT only ENET1 is used, so >>> ENET2 is >>> kept to the defaults (ENET2_TX_CLK_DIR = 0, ENET2_CLK_SEL = 0) and the >>> bootloader won't touch those bits. >> Ok, i see. It makes sense. >> >>>> With this bits we have following variants: >>>> 1. internal clock source with output on ENET1_TX_CLK >>>> 2. internal clock source without output on ENET1_TX_CLK. Are there any >>>> use cases need to support this mode? >>> After reading the reference manual, this mode refers to >>> ENET1_TX_CLK_DIR = >>> 0, ENET1_CLK_SEL = 0. Is my understanding correct? >>>> 3. external clock source without output on ENET1_TX_CLK >>>> 4. external clock source with output on ENET1_TX_CLK, well >>>> ENET1_TX_CLK >>>> is input it can't be out put on this case. >>>> >>>> Current kernel supports modes 1 and 3. For mode 2 I do not have a use >>>> case and mode 4 make no sense. >>>> >>>> In your case, the boot loader configures clocks to mode 2 which is not >>>> correct for this HW. It should be mode 1. >>> As written above the bootloader doesn't touch this. It's the reset >>> default >>> according to the reference manual. So i consider mode 2 as disabled >>> clock, >>> which is the right mode for boards without using this particular >>> Ethernet >>> interface. For EMC reasons we don't want to enable ENET1 and ENET2 >>> clock >>> output unconditionally. >>>> Probably, the way to go is do register dummy parents for not supported >>>> modes. It would silent the kernel. Other ideas? >>> Sorry, i don't have no idea how to properly achieve this. >> can you please test this patch: >> >> diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c >> index 2836adb817b7..e3696a88b5a3 100644 >> --- a/drivers/clk/imx/clk-imx6ul.c >> +++ b/drivers/clk/imx/clk-imx6ul.c >> @@ -95,14 +95,16 @@ static const struct clk_div_table >> video_div_table[] = { >> { } >> }; >> -static const char * enet1_ref_sels[] = { "enet1_ref_125m", >> "enet1_ref_pad", }; >> +static const char * enet1_ref_sels[] = { "enet1_ref_125m", >> "enet1_ref_pad", "dummy", "dummy"}; >> static const u32 enet1_ref_sels_table[] = { >> IMX6UL_GPR1_ENET1_TX_CLK_DIR, >> - IMX6UL_GPR1_ENET1_CLK_SEL }; >> + IMX6UL_GPR1_ENET1_CLK_SEL, 0, >> + IMX6UL_GPR1_ENET1_TX_CLK_DIR | >> IMX6UL_GPR1_ENET1_CLK_SEL }; >> static const u32 enet1_ref_sels_table_mask = >> IMX6UL_GPR1_ENET1_TX_CLK_DIR | >> IMX6UL_GPR1_ENET1_CLK_SEL; >> -static const char * enet2_ref_sels[] = { "enet2_ref_125m", >> "enet2_ref_pad", }; >> +static const char * enet2_ref_sels[] = { "enet2_ref_125m", >> "enet2_ref_pad", "dummy", "dummy"}; >> static const u32 enet2_ref_sels_table[] = { >> IMX6UL_GPR1_ENET2_TX_CLK_DIR, >> - IMX6UL_GPR1_ENET2_CLK_SEL }; >> + IMX6UL_GPR1_ENET2_CLK_SEL, 0, >> + IMX6UL_GPR1_ENET2_TX_CLK_DIR | >> IMX6UL_GPR1_ENET2_CLK_SEL }; >> static const u32 enet2_ref_sels_table_mask = >> IMX6UL_GPR1_ENET2_TX_CLK_DIR | >> IMX6UL_GPR1_ENET2_CLK_SEL; > > i successful tested this patch on top of Shawn's for-next branch. The > error message went away. > > Just 2 ideas for a proper patch: > > - short explaining comment in clk-imx6ul about the dummies > > - instead of "dummy" for both interfaces, i suggest something like > "enet1_ref_dummy" which makes investigation at > /sys/kernel/debug/clk/clk_summary easier please ignore the second point, because all the other clocks already uses "dummy" > Thanks > Stefan > >> Regards, >> Oleksij
--- clk_summary.good 2023-03-05 22:41:52.607392458 +0100 +++ clk_summary.bad 2023-03-05 22:41:52.587391697 +0100 @@ -46,12 +46,13 @@ usbphy2 0 0 0 480000000 0 0 50000 N pll6 1 1 0 500000000 0 0 50000 Y pll6_bypass 1 1 0 500000000 0 0 50000 Y - pll6_enet 2 2 0 500000000 0 0 50000 Y - enet_ptp_ref 1 1 0 25000000 0 0 50000 Y - enet_ptp 1 1 0 25000000 0 0 50000 Y + pll6_enet 1 1 0 500000000 0 0 50000 Y + enet_ptp_ref 0 0 0 25000000 0 0 50000 Y
Hi, we planned to submit our custom i.MX6ULL board [1] to mainline after release of Linux 6.3-rc1, but the recent enet refclock changes breaks our Ethernet phy: [ 0.000000] imx:clk-gpr-mux: failed to get parent (-EINVAL) ... [ 18.574595] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: phy_poll_reset failed: -110 [ 18.581064] fec 2188000.ethernet eth0: Unable to connect to phy I narrow down the PHY issue to this first bad commit: 5f82bfced611 ("clk: imx6ul: fix enet1 gate configuration") The clock issues seems to be cause by the following commit. If i revert 5f82bfced611 and 4e197ee880c24 or use Linux 6.2 everything is fine. Maybe this helps for further analyse: + enet_ptp 0 0 0 25000000 0 0 50000 N enet2_ref 0 0 0 50000000 0 0 50000 Y - enet_ref_125m 0 0 0 50000000 0 0 50000 N - enet_ref 3 3 0 50000000 0 0 50000 Y + enet2_ref_125m 0 0 0 50000000 0 0 50000 N + enet1_ref 1 1 0 50000000 0 0 50000 Y + enet1_ref_125m 0 0 0 50000000 0 0 50000 N pll5 0 0 0 296600000 0 0 50000 Y pll5_bypass 0 0 0 296600000 0 0 50000 Y pll5_video 0 0 0 296600000 0 0 50000 N @@ -137,19 +138,19 @@ step 0 0 0 528000000 0 0 50000 Y periph_pre 1 1 0 528000000 0 0 50000 Y periph 3 3 0 528000000 0 0 50000 Y - ahb 9 9 0 132000000 0 0 50000 Y + ahb 8 8 0 132000000 0 0 50000 Y sdma 4 1 0 132000000 0 0 50000 Y rom 1 1 0 132000000 0 0 50000 Y esai_mem 0 0 0 132000000 0 0 50000 N esai_ipg 0 0 0 132000000 0 0 50000 N aips_tz3 1 1 0 132000000 0 0 50000 Y - enet_ahb 1 1 0 132000000 0 0 50000 Y + enet_ahb 0 0 0 132000000 0 0 50000 N dcp 1 1 0 132000000 0 0 50000 Y asrc_mem 0 0 0 132000000 0 0 50000 N asrc_ipg 0 0 0 132000000 0 0 50000 N aips_tz2 1 1 0 132000000 0 0 50000 Y aips_tz1 1 1 0 132000000 0 0 50000 Y - ipg 15 13 0 66000000 0 0 50000 Y + ipg 14 12 0 66000000 0 0 50000 Y wdog3 0 0 0 66000000 0 0 50000 N uart8_ipg 0 0 0 66000000 0 0 50000 N usboh3 0 0 0 66000000 0 0 50000 N @@ -180,7 +181,7 @@ uart2_ipg 0 0 0 66000000 0 0 50000 N can2_ipg 0 0 0 66000000 0 0 50000 N can1_ipg 0 0 0 66000000 0 0 50000 N - enet 1 1 0 66000000 0 0 50000 Y + enet 0 0 0 66000000 0 0 50000 N axi_sel 1 1 0 528000000 0 0 50000 Y axi_podf 2 2 0 264000000 0 0 50000 Y axi 1 1 0 264000000 0 0 50000 Y [1] - https://github.com/chargebyte/linux/commit/5f47f1b029d34391a4b24d4a30105f517126363e#diff-a3def31334aa0f3b641ed5b21a60b462a9cbdda064e7276770be22d9860f2ae7R137