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
