imx6ul: Recent enet refclock changes breaks custom i.mx6ull board

[Stefan Wahren]

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