(sorry for resending, used wrong mail here - not sure how mailing list
will handle that).
Ian Campbell writes:
> On Wed, 2015-04-29 at 17:02 +0200, Daniel Kochmański wrote:
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index 88e3358..1a30684 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -239,6 +239,18 @@ config MMC_SUNXI_SLOT_EXTRA
>> slot or emmc on mmc1 - mmc3. Setting this to 1, 2 or 3 will enable
>> support for this.
>>
>> +config SPL_NAND_SUPPORT
>> + bool "SPL/NAND mode support"
>> + depends on SPL
>> + default n
>> + ---help---
>> + This enables support for booting from NAND internal
>> + memory. U-Boot SPL doesn't detect where is it load from,
>> + therefore this option is needed to properly load image from
>> + flash. Option also disables MMC functionality on U-Boot due to
>> + initialization errors encountered, when both controllers are
>> + enabled.
>
> Is this last bit a bug in the s/w or a hardware thing? Does this mean
> that MMC is not available in the main u-boot image too when NAND support
> is enabled?
>
I'm not sure what is a reason, but controller can't initialize MMC when
SPL is loaded. MMC works in main u-boot allowing to boot from it and
perform any other option (rescan, part, info, etc.). It is only SPL
relevant.
>> +
>> config USB0_VBUS_PIN
>> string "Vbus enable pin for usb0 (otg)"
>> default ""
>> diff --git a/board/sunxi/Makefile b/board/sunxi/Makefile
>> index 43766e0..7ad7412 100644
>> --- a/board/sunxi/Makefile
>> +++ b/board/sunxi/Makefile
>> @@ -9,6 +9,7 @@
>> # SPDX-License-Identifier: GPL-2.0+
>> #
>> obj-y += board.o
>> +obj-$(CONFIG_NAND_SUNXI) += nand.o
>
> CONFIG_SUNXI_NAND would be more consistent with the rest I think.
>
OK.
>> obj-$(CONFIG_SUNXI_GMAC) += gmac.o
>> obj-$(CONFIG_SUNXI_AHCI) += ahci.o
>
>> +void nand_set_clocks(void)
>> +{
>> + W32(PORTC_BASE + 0x48, 0x22222222);
>
> u-boot style is to declare a struct which matches the register layout
> and to do something like:
> struct nand_ctrl_foo *mcf = (void *)NANCFLASHC_BASE;
>
> And use writel, set_bits, setclr_bits and friends.
OK, I'll read about it and try to adjust driver.
>
> Please also try and give sensible names to all the magic numbers, or at
> least include a comment explaining that these are magic numbers derived
> from $SOMEWHERE and we don't know what they mean (if that is the case).
>
> Both of these apply to several bits of the code too.
This work is mainly based on register guess-work from here:
http://rhombus-tech.net/allwinner_a10/A10_register_guide/A10_NAND/ - so
I can gather names from there and put them in source.
>
>> + W32(PORTC_BASE + 0x4C, 0x22222222);
>> + W32(PORTC_BASE + 0x50, 0x2222222);
>> + W32(PORTC_BASE + 0x54, 0x2);
>> + W32(PORTC_BASE + 0x5C, 0x55555555);
>> + W32(PORTC_BASE + 0x60, 0x15555);
>> + W32(PORTC_BASE + 0x64, 0x5140);
>> + W32(PORTC_BASE + 0x68, 0x4016);
>> +
>> + uint32_t val = R32(CCMU_BASE + 0x60);
>> + W32(CCMU_BASE + 0x60, 0x2000 | val);
>> + val = R32(CCMU_BASE + 0x80);
>> + W32(CCMU_BASE + 0x80, val | 0x80000000 | 0x1);
>> +}
>> +
>> +int initialized = 0;
>> +void nand_init(void)
>> +{
>> + initialized = 1;
>
> Please call this from somewhere during init e.g. board.c rather than
> from nand_read with a latch.
>
Sure.
>> + uint32_t val;
>> + nand_set_clocks();
>> + val = R32(NANDFLASHC_BASE + 0x00);
>> + /* CTL = (1<<0 <-EN 1<<1 RESET) */
>> + W32(NANDFLASHC_BASE + 0x00, val | 0x3); /* enable and reset CTL */
>> + do {
>> + val = R32(NANDFLASHC_BASE + 0x00);
>> + if (val & (1<<1))
>> + break;
>> + } while (1);
>
> Potentially infinite loop?
>
> There were some similar instances below which had a t/o. Perhaps combine
> them all into a helper similar to the dram code's
> mctl_await_completion()?
>
Ok, I'll look into that.
>> +uint32_t ecc_errors = 0;
>> +
>> +/* read 0x400 bytes from real_addr to temp_buf */
>> +void nand_read_block(unsigned int real_addr, int syndrome)
>> +{
>> + uint32_t val;
>> + if (!initialized)
>> + nand_init();
>> +
>> + memset((void *)temp_buf, 0, 0x400); /* clear temp_buf */
>
> Can we not avoid going via this global temp_buf by setting DMAC_BASE +
> 0x308 to the correct address on each read?
>
I'll check this, I think I had problems with that, but if it works w/o
temp_buf on both SPL and main U-Boot builds, then I'll get rid of it.
> Ian.
--
Daniel Kochmański | Poznań, Poland
;; aka jackdaniel
"Be the change that you wish to see in the world." - Mahatma Gandhi
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
