On Tue, May 07, 2019 at 11:39:12AM +0200, Niel Fourie wrote: > Hi Tom, > > On 5/6/19 7:24 PM, Tom Rini wrote: > >On Mon, May 06, 2019 at 06:44:48PM +0200, Niel Fourie wrote: > >>Hi Tom, > >> > >>On 5/6/19 4:18 PM, Tom Rini wrote: > >>>On Mon, May 06, 2019 at 04:02:53PM +0200, Niel Fourie wrote: > >>> > >>>>Support for Phytech phyCORE AM335x R2 SOM (PCL060) on the Phytec > >>>>phyBOARD-Wega AM335x. > >>>> > >>>>CPU : AM335X-GP rev 2.1 > >>>>Model: Phytec AM335x phyBOARD-WEGA > >>>>DRAM: 256 MiB > >>>>NAND: 256 MiB > >>>>MMC: OMAP SD/MMC: 0 > >>>>eth0: ethernet@4a100000 > >>>> > >>>>Working: > >>>> - Eth0 > >>>> - i2C > >>>> - MMC/SD > >>>> - NAND > >>>> - UART > >>>> - USB (host) > >>>> > >>>>Device trees were taken from Linux mainline: > >>>>commit 37624b58542f ("Linux 5.1-rc7") > >>>> > >>>>Signed-off-by: Niel Fourie <lu...@denx.de> > [snip] > >> > >>void sdram_init(void) > >>{ > >> /* Configure memory to maximum supported size for detection */ > >> int ram_type_index = PHYCORE_R2_MT41K512M16HA125IT_1024MB; > >> config_ddr(DDR_CLK_MHZ, &ioregs, > >> &physom_timings[ram_type_index].ddr3_data, > >> &ddr3_cmd_ctrl_data, > >> &physom_timings[ram_type_index].ddr3_emif_reg_data, > >> 0); > >> > >> /* Detect memory physically present */ > >> gd->ram_size = get_ram_size((void *)CONFIG_SYS_SDRAM_BASE, > >> CONFIG_MAX_RAM_BANK_SIZE); > >> > >> /* Reconfigure memory for actual detected size */ > >> switch (gd->ram_size) { > >> case SZ_1G: > >> ram_type_index = PHYCORE_R2_MT41K512M16HA125IT_1024MB; > >> break; > >> case SZ_512M: > >> ram_type_index = PHYCORE_R2_MT41K256M16TW107IT_512MB; > >> break; > >> case SZ_256M: > >> default: > >> ram_type_index = PHYCORE_R2_MT41K128M16JT_256MB; > >> break; > >> } > >> config_ddr(DDR_CLK_MHZ, &ioregs, > >> &physom_timings[ram_type_index].ddr3_data, > >> &ddr3_cmd_ctrl_data, > >> &physom_timings[ram_type_index].ddr3_emif_reg_data, > >> 0); > >>} > >> > >>The ugliest part of this is, as you pointed out, that directly after this is > >>called, get_ram_size() will be called again from sdram_init(). But it at > >>least noninvasive, and no longer requires the device tree. > > > >I don't think it's safe to call config_ddr twice, especially with the > >possibly wrong parameters. What's barebox doing in this case, being > >told the presumably correct DDR size in the device tree? > > Good point. Barebox uses the above mechanism to detect the memory size, and > I could find no equivalent memory size specified in its internal device > tree.
Configure for 1GB and then see how much we can actually talk to? > Marek originally proposed using the memory size specified in the device tree > as an improvement over specifying the size in the defconfig (as in v1 of the > patch). But then you aren't populating 3 device trees nor making it clear / easy to say which module you're on, and then still need to change the config for which DT you're picking up. These SOMs really don't provide any run-time method to see which one you're on? There's no GPIOs to poke? -- Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot