[RFC] Vocore2 MMC needs clock patches
Hi Weijie, I attach my, apparently working, port to VoCore2 SoM.
These patchsets are on top of Your 21 patch rewrite of MT7628 board,
While the first patchset is relatively straightforward, but does not include MMC handling, to enable it I had to backport from Linux Kernel several pieces, essentially clock and interrupt handling.
Those drivers appear written by You. I am unsure if this is really needed or if there is some other (perhaps cleaner) way to enable MMC.
As said this seems to work for me, but I would like to contribute this board upstream, in the best possible way.
Please let me know how I should proceed.
Best Regards and Thanks in Advance
On Tue, 2020-01-21 at 00:55 +0100, Mauro Condarelli wrote:
Hi Weijie, I attach my, apparently working, port to VoCore2 SoM.
These patchsets are on top of Your 21 patch rewrite of MT7628 board,
While the first patchset is relatively straightforward, but does not include MMC handling, to enable it I had to backport from Linux Kernel several pieces, essentially clock and interrupt handling.
You are using a superseded patch series (v1) I submitted several months ago. These patches have already been replaced by v3 and merged into the mainline.
* "mtmips-clk-gate" from v1 is replaced by "mediatek,mt7628-clk" and the function of "mediatek,mt7628-clk" is a superset of "mtmips-clk-gate". * The node "intc" has no use at all. U-Boot for mips use no interrupts. * mmc property "hclk" is the gating clock of the SD controller module. Using <&clk48m> in the v1 patches was a bad idea. I changed it to <&clkctrl CLK_SDXC> in v3 to make sure its clock will be always enabled. * Please move pinctrl properties to your board's dts file. * pinctrl name "state_uhs" is not used by mtk-sd in U-Boot. MT7628 does not support UHS. You should remove it. * vmmc-supply and vqmmc-supply are not used by mtk-sd in U-Boot because UHS support is not added to the driver. Besides you have assigned wrong values to them. You should remove it.
Those drivers appear written by You. I am unsure if this is really needed or if there is some other (perhaps cleaner) way to enable MMC.
As said this seems to work for me, but I would like to contribute this board upstream, in the best possible way.
Please let me know how I should proceed.
I don't have a board with the "sd_iot_mode" pinmux for SDXC, so I can't tell you why you can't use it with my v3 patches. But I have tested v3 patches on boards using "sd_router_mode" pinmux and they do work well.
Best Regards and Thanks in Advance
Thanks Weijie, I made the changes You suggested. I have also seen You sent a new version of Your patches. Since mine are based on yours I *think* I should suspend sending my VoCore2 patches till Yours are fixed and integrated into master.
@Stefan Roese: is this the right course of action?
I am happy what I have (pending further test, of course!), but I want this integrated in upstream master, if possible, so I'm prepared to rebase my patches after Weijie ones are in.
Any comment welcome.
Side Question: Stefan wrote:
Most of this can be done by using the RAM version now (again). There is no additional RAM booting target now any more. You can use the normal U-Boot image for this now. Please note the changes TEXT_BASE here. Its now 0x80200000.
This actually seems to work right if I start from my original u-boot (1.1.3, flashed at start of SPI NOR), but it fails if I start from a flashed (at the same location) u-boot-mtmips.bin I *think* this happens because unpacking actually writes u-boot at 0x80200000 and runs it from there, so "load usb 0:1 80200000 u-boot.bin" (or equivalent) will overwrite the running u-boot and the following "go ${fileaddr}" fails: ## Starting application at 0x80200000 ... <DEAD> Note that if I load the same version flashed it works ok (presumably because I'm overwriting with the same content, so no harm done). How can I load in RAM end test a new version once I flash new-style u-boot? I am thinking about relinking at a different address, but I'm unsure.
TiA!
Regards Mauro Condarelli
On 1/21/20 4:11 AM, Weijie Gao wrote:
On Tue, 2020-01-21 at 00:55 +0100, Mauro Condarelli wrote:
Hi Weijie, I attach my, apparently working, port to VoCore2 SoM.
These patchsets are on top of Your 21 patch rewrite of MT7628 board,
While the first patchset is relatively straightforward, but does not include MMC handling, to enable it I had to backport from Linux Kernel several pieces, essentially clock and interrupt handling.
You are using a superseded patch series (v1) I submitted several months ago. These patches have already been replaced by v3 and merged into the mainline.
- "mtmips-clk-gate" from v1 is replaced by "mediatek,mt7628-clk" and the function of "mediatek,mt7628-clk" is a superset of "mtmips-clk-gate".
- The node "intc" has no use at all. U-Boot for mips use no interrupts.
- mmc property "hclk" is the gating clock of the SD controller module. Using <&clk48m> in the v1 patches was a bad idea. I changed it to <&clkctrl CLK_SDXC> in v3 to make sure its clock will be always enabled.
- Please move pinctrl properties to your board's dts file.
- pinctrl name "state_uhs" is not used by mtk-sd in U-Boot. MT7628 does not support UHS. You should remove it.
- vmmc-supply and vqmmc-supply are not used by mtk-sd in U-Boot because UHS support is not added to the driver. Besides you have assigned wrong values to them. You should remove it.
Those drivers appear written by You. I am unsure if this is really needed or if there is some other (perhaps cleaner) way to enable MMC.
As said this seems to work for me, but I would like to contribute this board upstream, in the best possible way.
Please let me know how I should proceed.
I don't have a board with the "sd_iot_mode" pinmux for SDXC, so I can't tell you why you can't use it with my v3 patches. But I have tested v3 patches on boards using "sd_router_mode" pinmux and they do work well.
Best Regards and Thanks in Advance
Hi Mauro,
On 21.01.20 12:27, Mauro Condarelli wrote:
Thanks Weijie, I made the changes You suggested. I have also seen You sent a new version of Your patches. Since mine are based on yours I *think* I should suspend sending my VoCore2 patches till Yours are fixed and integrated into master.
@Stefan Roese: is this the right course of action?
I think in the current state of Weijie's patches (v3), you can resume sending your VoCore2 support based on this latest patchset to the list. Please don't attach a patch but send it inline next time (git send-email) to enable review.
I am happy what I have (pending further test, of course!), but I want this integrated in upstream master, if possible, so I'm prepared to rebase my patches after Weijie ones are in.
Any comment welcome.
Side Question: Stefan wrote:
Most of this can be done by using the RAM version now (again). There is no additional RAM booting target now any more. You can use the normal U-Boot image for this now. Please note the changes TEXT_BASE here. Its now 0x80200000.
This actually seems to work right if I start from my original u-boot (1.1.3, flashed at start of SPI NOR), but it fails if I start from a flashed (at the same location) u-boot-mtmips.bin I *think* this happens because unpacking actually writes u-boot at 0x80200000 and runs it from there, so "load usb 0:1 80200000 u-boot.bin" (or equivalent) will overwrite the running u-boot and the following "go ${fileaddr}" fails: ## Starting application at 0x80200000 ... <DEAD>
No, U-Boot relocates itself to the end of RAM and runs from there. So this should work. Perhaps a cache flush is missing.
I'll give it a try on my LinkIt board as well later.
Note that if I load the same version flashed it works ok (presumably because I'm overwriting with the same content, so no harm done). How can I load in RAM end test a new version once I flash new-style u-boot? I am thinking about relinking at a different address, but I'm unsure.
See above.
Thanks, Stefan
Hi Stefan,
On 1/21/20 1:08 PM, Stefan Roese wrote:
Hi Mauro,
On 21.01.20 12:27, Mauro Condarelli wrote:
Thanks Weijie, I made the changes You suggested. I have also seen You sent a new version of Your patches. Since mine are based on yours I *think* I should suspend sending my VoCore2 patches till Yours are fixed and integrated into master.
@Stefan Roese: is this the right course of action?
I think in the current state of Weijie's patches (v3), you can resume sending your VoCore2 support based on this latest patchset to the list. Please don't attach a patch but send it inline next time (git send-email) to enable review.
I will send next iteration as soon as I fix the reflash problem (see below). ===8<----
Side Question: Stefan wrote:
Most of this can be done by using the RAM version now (again). There is no additional RAM booting target now any more. You can use the normal U-Boot image for this now. Please note the changes TEXT_BASE here. Its now 0x80200000.
This actually seems to work right if I start from my original u-boot (1.1.3, flashed at start of SPI NOR), but it fails if I start from a flashed (at the same location) u-boot-mtmips.bin I *think* this happens because unpacking actually writes u-boot at 0x80200000 and runs it from there, so "load usb 0:1 80200000 u-boot.bin" (or equivalent) will overwrite the running u-boot and the following "go ${fileaddr}" fails: ## Starting application at 0x80200000 ... <DEAD>
No, U-Boot relocates itself to the end of RAM and runs from there. So this should work. Perhaps a cache flush is missing.
I'll give it a try on my LinkIt board as well later.
Did You manage to test this? I am currently testing loading from "paleolithic" u-boot, but I want to fix this before I finalize VoCore2 patches.
I tried to do some manual testing enabling CONFIG_CMD_CACHE, but this bombs with Weijie patches with:
LD u-boot mipsel-linux-ld.bfd: cmd/built-in.o: in function `do_icache': cmd/cache.c:(.text.do_icache+0x5c): undefined reference to `icache_disable' mipsel-linux-ld.bfd: cmd/cache.c:(.text.do_icache+0x6c): undefined reference to `icache_enable' mipsel-linux-ld.bfd: cmd/cache.c:(.text.do_icache+0x8c): undefined reference to `icache_status' make: *** [Makefile:1697: u-boot] Error 1
icache seems enabled unconditionally in arch/mips/mach-mtmips/mt7628/lowlevel_init.S::73+
I will try to add dummy functions just-to-play.
Please advise
Thanks, Stefan
Regards and Many Thanks Mauro
Hi, I'm clearly out of my depth:
=> icache flush No arch specific invalidate_icache_all available! => dcache flush No arch specific flush_dcache_all available! => icache off => dcache off
Ooops: $ 0 : 00000000 00000000 87e806c8 00000000 $ 4 : ffffffd0 00000014 87e80518 87fce61c $ 8 : 87e7bc00 00000010 00000000 fffffffc $12 : 00000000 0000000f 00000006 00000007 $16 : ffffffd0 00000000 00000000 0000002f $20 : 87e81350 87fe8528 00000000 00000001 $24 : 00000016 87f84130 $28 : 87f882a0 87e7bba8 87e81362 87fa9524 Hi : 00000006 Lo : 000640a2 epc : 87fa1600 (text 80221600) ra : 87fa9524 (text 80229524) Status: 00000006 Cause : d0008028 (ExcCode 0a) PrId : 00019655 ### ERROR ### Please RESET the board ###
Thanks Mauro
On 1/22/20 9:16 PM, Mauro Condarelli wrote:
Hi Stefan,
On 1/21/20 1:08 PM, Stefan Roese wrote:
Hi Mauro,
On 21.01.20 12:27, Mauro Condarelli wrote:
Thanks Weijie, I made the changes You suggested. I have also seen You sent a new version of Your patches. Since mine are based on yours I *think* I should suspend sending my VoCore2 patches till Yours are fixed and integrated into master.
@Stefan Roese: is this the right course of action?
I think in the current state of Weijie's patches (v3), you can resume sending your VoCore2 support based on this latest patchset to the list. Please don't attach a patch but send it inline next time (git send-email) to enable review.
I will send next iteration as soon as I fix the reflash problem (see below). ===8<----
Side Question: Stefan wrote:
Most of this can be done by using the RAM version now (again). There is no additional RAM booting target now any more. You can use the normal U-Boot image for this now. Please note the changes TEXT_BASE here. Its now 0x80200000.
This actually seems to work right if I start from my original u-boot (1.1.3, flashed at start of SPI NOR), but it fails if I start from a flashed (at the same location) u-boot-mtmips.bin I *think* this happens because unpacking actually writes u-boot at 0x80200000 and runs it from there, so "load usb 0:1 80200000 u-boot.bin" (or equivalent) will overwrite the running u-boot and the following "go ${fileaddr}" fails: ## Starting application at 0x80200000 ... <DEAD>
No, U-Boot relocates itself to the end of RAM and runs from there. So this should work. Perhaps a cache flush is missing.
I'll give it a try on my LinkIt board as well later.
Did You manage to test this? I am currently testing loading from "paleolithic" u-boot, but I want to fix this before I finalize VoCore2 patches.
I tried to do some manual testing enabling CONFIG_CMD_CACHE, but this bombs with Weijie patches with:
LD u-boot mipsel-linux-ld.bfd: cmd/built-in.o: in function `do_icache': cmd/cache.c:(.text.do_icache+0x5c): undefined reference to `icache_disable' mipsel-linux-ld.bfd: cmd/cache.c:(.text.do_icache+0x6c): undefined reference to `icache_enable' mipsel-linux-ld.bfd: cmd/cache.c:(.text.do_icache+0x8c): undefined reference to `icache_status' make: *** [Makefile:1697: u-boot] Error 1
icache seems enabled unconditionally in arch/mips/mach-mtmips/mt7628/lowlevel_init.S::73+
I will try to add dummy functions just-to-play.
Please advise
Thanks, Stefan
Regards and Many Thanks Mauro
Hi Maruo,
On 22.01.20 21:16, Mauro Condarelli wrote:
Hi Stefan,
On 1/21/20 1:08 PM, Stefan Roese wrote:
Hi Mauro,
On 21.01.20 12:27, Mauro Condarelli wrote:
Thanks Weijie, I made the changes You suggested. I have also seen You sent a new version of Your patches. Since mine are based on yours I *think* I should suspend sending my VoCore2 patches till Yours are fixed and integrated into master.
@Stefan Roese: is this the right course of action?
I think in the current state of Weijie's patches (v3), you can resume sending your VoCore2 support based on this latest patchset to the list. Please don't attach a patch but send it inline next time (git send-email) to enable review.
I will send next iteration as soon as I fix the reflash problem (see below).
===8<----
Side Question: Stefan wrote:
Most of this can be done by using the RAM version now (again). There is no additional RAM booting target now any more. You can use the normal U-Boot image for this now. Please note the changes TEXT_BASE here. Its now 0x80200000.
This actually seems to work right if I start from my original u-boot (1.1.3, flashed at start of SPI NOR), but it fails if I start from a flashed (at the same location) u-boot-mtmips.bin I *think* this happens because unpacking actually writes u-boot at 0x80200000 and runs it from there, so "load usb 0:1 80200000 u-boot.bin" (or equivalent) will overwrite the running u-boot and the following "go ${fileaddr}" fails: ## Starting application at 0x80200000 ... <DEAD>
No, U-Boot relocates itself to the end of RAM and runs from there. So this should work. Perhaps a cache flush is missing.
I'll give it a try on my LinkIt board as well later.
Did You manage to test this?
Yes. I just tested for a few minutes and it seems to work on the LinkIt board:
=> printenv tt tt=tftp 80200000 ${tftpdir}/u-boot.bin;dcache off;go ${fileaddr} => run tt Using eth@10110000 device TFTP from server 192.168.1.5; our IP address is 192.168.1.233 Filename 'linkit-smart-7688/u-boot.bin'. Load address: 0x80200000 Loading: ############################### 4.8 MiB/s done Bytes transferred = 450139 (6de5b hex) Unknown command 'dcache' - try 'help' ## Starting application at 0x80200000 ...
U-Boot 2020.01-00680-g90cb39245e (Jan 21 2020 - 10:54:50 +0100)
CPU: MediaTek MT7688A ver:1 eco:2 Boot: DDR2, SPI-NOR 4-Byte Addr, CPU clock from XTAL Clock: CPU: 580MHz, Bus: 193MHz, XTAL: 40MHz Model: LinkIt-Smart-7688 ...
I am currently testing loading from "paleolithic" u-boot, but I want to fix this before I finalize VoCore2 patches.
I tried to do some manual testing enabling CONFIG_CMD_CACHE, but this bombs with Weijie patches with:
LD u-boot mipsel-linux-ld.bfd: cmd/built-in.o: in function `do_icache': cmd/cache.c:(.text.do_icache+0x5c): undefined reference to `icache_disable' mipsel-linux-ld.bfd: cmd/cache.c:(.text.do_icache+0x6c): undefined reference to `icache_enable' mipsel-linux-ld.bfd: cmd/cache.c:(.text.do_icache+0x8c): undefined reference to `icache_status' make: *** [Makefile:1697: u-boot] Error 1
icache seems enabled unconditionally in arch/mips/mach-mtmips/mt7628/lowlevel_init.S::73+
I will try to add dummy functions just-to-play.
No need to "play" with these cache functions. Its included in the LinkIt image and should be in yours as well, if you didn't change too much.
Thanks, Stefan
participants (3)
-
Mauro Condarelli -
Stefan Roese -
Weijie Gao