[Letux-kernel] ingenic-pinctrl (Memory freeze and timing CI20)

Sat Feb 27 07:03:36 CET 2021

Hi Nikolaus,

On 2021/2/25 下午9:18, H. Nikolaus Schaller wrote:
> Hi,
>
>> Am 24.02.2021 um 17:21 schrieb Zhou Yanjie <zhouyanjie at wanyeetech.com>:
>>
>> Hello Nikolaus and Paul,
>>
>>
>> I have completed a new pinctrl patches (in the attachment) based on kernel 5.11, including JZ4730, JZ4750, JZ4755, JZ4775 and X2000.
>>
>> The JZ4730 is based on Paul's driver with some minor adjustments.
> I tried to figure out what has been adjusted but it is difficult to tell... At least it is not obvious :)

The main change is to adjust the order of ID to make it more in line 
with the order of the birth of the chip, and adjust the order of 
corresponding conditional judgments to unify the code style.

>> Could you help test it? I don't have JZ4730 hardware at hand.
> Sure. I have reverted the jz470 pinctrl patches from the letux-5.11 tree and added yours and could not identify a functional difference so far.
>
> I can boot. SD card is working, LCD is working, I2C is generally working, keyboard scanning works, LEDs are blinking etc. and
> these were the most problematic components which rely on pinctrl.
>
> And I have checked by adding a printk() to probe that my device really runs the new driver...

Nice to hear it, thanks for your test.

>> Also, Paul, would you please give me your signed-off-by? I will add it when I send the patch.
> If I am right, you did take the latest (squashed) version from
>
> https://git.goldelico.com/?p=letux-kernel.git;a=commit;h=73d679d17ebcdd4457033e3e490471d397ad4b42
>
> which merges work by Paul and me. It already has our both "signed-off-by" so you can continue on this.
>
> Maybe you can add:
>
> Signed-off-by: Paul Boddie <paul at boddie.org.uk>	# for jz4730
> Signed-off-by: H. Nikolaus Schaller <hns at goldelico.com>	# for jz4730
> Tested-by: H. Nikolaus Schaller <hns at goldelico.com>	# on Letux400 / jz4730

Sure, I will add them.

> Generally:
>
> 1. there are some additional patches in our tree:
>
> https://git.goldelico.com/?p=letux-kernel.git;a=blobdiff;f=Documentation/devicetree/bindings/pinctrl/ingenic%2Cpinctrl.yaml;h=940e8681ce88537812c326fd2a39d6253d6bc47b;hp=44c04d11ae4ccddf84d8bdd1be681523cde173dd;hb=5f86f6f3c5f7a035770ef23af4f468b0b83dd516;hpb=0a59c58b515ca516b8d938df47c1876f43d185a3

Well, this is already in the patch.

> and
>
> https://git.goldelico.com/?p=letux-kernel.git;a=commit;h=15b615f8ac53ea118bc2c728d2d4ef778bc35550

This has not been included in the patch. I have found that the pinctrl 
code in the next tree has new changes compared to kernel 5.11. I am 
going to make a new patch after kernel 5.12-rc1 comes out, and I will 
add it to the new patch.

> I have not checked if these changes are included in your patch set.
>
> 2. jz4730 is special in having two control registers (upper/lower).
> This was mentioned in Paul's original commit message:
>
> 	Also add code to handle the jz4730 specific register offsets
> 	and some register pairs have 2 bits for each GPIO pin.
>
> Maybe you can add this to the [PATCH 3/3] description.

Sure, I will add it.

> 3. I had planned to submit our driver patch set soon which would be in conflict.
> The reason I haven't done yet is that I am not happy with the jz4730 i2c driver
> (it seems to be impossible to use interrups with this i2c controller hardware
> and the non-irq version times out or even hangs the i2 bus in one of my test
> cases).
>
> So it is better if your series goes upstream because I do not want to slow it
> down. And it might even help to upstream the rest of my jz4730 series if we
> already get some jz4730 support reviewed and merged.

Sure, I plan to submit the first version next week.

> BR and thanks,
> Nikolaus
>
>
>
>>
>> There is also bad news. I found that the previous cache patch may affect the normal operation of the USB camera. I am troubleshooting the problem.
> That is strange how the cache can influence USB operation. More memory wear than usual?

Ingenic's XBurst1 processor core has some special hardware to provide 
maintenance of L2 cache, which can provide better performance than 
software maintenance of L2 cache, but unfortunately there may be some 
problem with the support of some cache instructions, which causes errors 
when USB camera to acquire images (specifically reflected in there has 
abnormal color blocks in the acquired image).

Thanks and best regards!

>>
>> Thanks and best regards!
>>
>>
>> On 2021/2/16 上午4:07, H. Nikolaus Schaller wrote:
>>>> Am 15.02.2021 um 20:56 schrieb Zhou Yanjie <zhouyanjie at wanyeetech.com>:
>>>>
>>>> Hi Nikolaus,
>>>>
>>>> On 2021/2/15 下午7:12, H. Nikolaus Schaller wrote:
>>>>> Hi,
>>>>> sorry for the delay on my side now...
>>>>>
>>>>>> Am 11.02.2021 um 16:15 schrieb Zhou Yanjie <zhouyanjie at wanyeetech.com>:
>>>>>>
>>>>>> Hi Nikolaus,
>>>>>>
>>>>>>
>>>>>> Sorry, I got caught up in other thins last few weeks.
>>>>>>
>>>>>>
>>>>>> On 2021/2/5 下午2:53, H. Nikolaus Schaller wrote:
>>>>>>> Hi,
>>>>>>> I got a hint from a member of the Pyra community who had been following
>>>>>>> the OMAP5 + DDR3 timing issues, that the jz4780/CI20 may have similar issues.
>>>>>> Could you describe the specific phenomenon?
>>>>> I do not know more than described in the pull request description
>>>>>
>>>>>>> https://github.com/MIPS/CI20_u-boot/pull/18
>>>>> The author says
>>>>>
>>>>> "This fixes a few timing errors in the memory initialization routine, specifically the minimum self-refresh time which was too low for both Samsung and Hynix chip, as well as the four bank activation time and the exit-power-down-to-active-command delay for the Samsung ones. The changes are based on the memory chips' most recent datasheets as well as the JZ4780 manual and have fixed all memory-related freezes I've been experiencing with my board."
>>>>>
>>>>> But I haven't observed memory related issues myself. BTW it is not clear how to
>>>>> easily distinguish that from other freezes.
>>>>>
>>>>> What we had was a memory timing issue on the omap5. There stability did clearly depend
>>>>> on the installed memory chips (there were two variants) and tweaking the memory timings
>>>>> did fix it for all. It had something to do with automatic timing measurements and corrections
>>>>> of the DDR3 chips. This was not enabled correctly on omap5.
>>>> I'm glad to hear you fixed the timing issue,
>>> well to be fair it was Tony Lindgren, the OMAP kenrel maintainer who fixed this for OMAP5.
>>>
>>> And I just was notified that a similar fix seems to exist for the CI20 board.
>>>
>>>> but unfortunately I am not familiar with the memory timing training.
>>> I am not either.
>>>
>>>> The parameters in u-boot I transplanted are all provided by Ingenic directly. I may not be able to help much in this part.
>>> Well, we used the old 2013 u-boot which also seems to orgin in Ingenic.
>>>
>>> So I am not even sure if this is a real problem.
>>>
>>>>>>   And witch version u-boot do you use?
>>>>> I use this u-boot:
>>>>>
>>>>> https://git.goldelico.com/?p=letux-uboot.git;a=shortlog;h=refs/heads/ci20-v2013.10
>>>>>
>>>>> seems to be a 2013.10 u-boot with a lot of fixes up to 2017.
>>>> I am also using this version, and have not encountered timing issues for the time being.
>>> Me too. So it may be a false alarm that a fix is needed.
>>>
>>>>>> It seems that now the u-boot mainline (after version 2019.01) also supports CI20.
>>>>> So it could be that the mainline u-boot already includes the patch
>>>>>
>>>>> https://github.com/MIPS/CI20_u-boot/pull/18/commits/3c4bdbd749edc344abf11823282363d79c1d5eeb
>>>> I'm not too sure. To be honest, I haven't read the mainline u-boot code of CI20 carefully. I am trying to port the u-boot of X1000 and X1830 from the 2013.07 version (provided by Ingenic) to the 2015.04 version to provide support for FIT.
>>> Ah, interesting.
>>>
>>> BTW: Lobomir Rintel did make a mainline u-boot for the jz4730 which we use as second stage u-boot to be able to load modern kernels (there is a very very old u-boot in NAND flash which is limited in gzip file size for unpacking the kernel):
>>>
>>> https://git.goldelico.com/?p=letux-uboot.git;a=shortlog;h=refs/heads/work/hns/mipsbook
>>>
>>> Maybe this is helpful in any way.
>>>
>>>>>>> I am not sure if it is needed or not since my board works well without - except
>>>>>>> the freezes I had observed with SMP but I did not relate this to memory timing.
>>>>>>>
>>>>>>> There is a patch floating around:
>>>>>>>
>>>>>>> https://github.com/MIPS/CI20_u-boot/pull/18
>>>>>>>
>>>>>>> I have checked an in my U-Boot it is not included but could be added.
>>>>>>>
>>>>>>> What do you think?
>>>>>>>
>>>>>>> BTW: what is the status of the SMP / Cache driver?
>>>>>> It seems to work normally based on 5.10.7, I will send it to you later.
>>>>> Fine and thanks!
>>>> The new patch is in the attachment, you only need to replace thee original [10/13] patch with it. So far, there is no abnormality in the test of X1000 and X1830, and there is also no abnormality in the single-core test of JZ4780, the dual-core test has not yet been completed.
>>>>
>>>>
>>>> Thanks and best regards!
>>> I'll apply when the build of v5.11 (and some other LTS kernels) is done (which takes more and more time with every release :).
>>>
>>> Thanks and BR,
>>> Nikolaus
>> <0001-pinctrl-Ingenic-Fix-bug-and-reformat-the-code.patch><0002-dt-bindings-pinctrl-Add-bindings-for-new-Ingenic-SoC.patch><0003-pinctrl-Ingenic-Add-support-for-new-Ingenic-SoCs.patch>