Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
On 07.05.19 16:43, Niel Fourie wrote:
> Hi Tom,
>
> On 5/7/19 3:19 PM, Tom Rini wrote:
>> 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@4a10
>>>
>>> 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
>>> [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, ,
> _timings[ram_type_index].ddr3_data,
> _cmd_ctrl_data,
> _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, ,
> _timings[ram_type_index].ddr3_data,
> _cmd_ctrl_data,
> _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?
>
> Yes. If you are interested, you can see their implementation here [1],
> where get_minimal_timings() returns the configuration for 1GiB;
> everything is in that file. (I did credit the author.)
>
> [1]
> https://git.pengutronix.de/cgit/barebox/tree/arch/arm/boards/phytec-som-am335x/lowlevel.c#n167
>
>>> 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?
>
> Agreed, the device tree solution is inferior to autodetection. The
> SOMs manual makes no mention of how different variants can be
> distinguished/detected, and the board specific code in Barebox
> (written by Phytec) does not contain any other detection code (except
> for the RAM), like checking GPIOs. Unfortunately there is no publicly
> available schematic, so I can't be completely sure. So I am going to
> assume it, there is no other way of detection.
You are right, for our AM335 based SOMs there is no way to detect the
variant. The barebox generates for every possible RAM configuration an
own MLO. The nice thing is that barebox generates all variants with one
build by using its multi image mechanism.
Besides the RAM, the phyCORE can be populated with different AM335x
variants, NANDs, RTC/EEPROM/Eth-PHY(yes/no) and maybe other things I
don't remember now.
I think it would be a good Idea to maintain a list of article numbers
(that fully describes the SOM variant) which are supported by the
current u-boot version.
Regards,
Wadim
>
> Best regards,
> Niel Fourie
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
On Tue, May 07, 2019 at 04:43:04PM +0200, Niel Fourie wrote:
> Hi Tom,
>
> On 5/7/19 3:19 PM, Tom Rini wrote:
> >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@4a10
> >>
> >>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
> >>[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, ,
> _timings[ram_type_index].ddr3_data,
> _cmd_ctrl_data,
> _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, ,
> _timings[ram_type_index].ddr3_data,
> _cmd_ctrl_data,
> _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?
>
> Yes. If you are interested, you can see their implementation here [1], where
> get_minimal_timings() returns the configuration for 1GiB; everything is in
> that file. (I did credit the author.)
>
> [1]
> https://git.pengutronix.de/cgit/barebox/tree/arch/arm/boards/phytec-som-am335x/lowlevel.c#n167
Thinking back to my time at TI, that leaves me a little worried, but if
the vendor is OK with that approach, I'll leave it be, and yes, we can
mirror that approach here. Thanks for the links!
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
Hi Tom,
On 5/7/19 3:19 PM, Tom Rini wrote:
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@4a10
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
[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, ,
_timings[ram_type_index].ddr3_data,
_cmd_ctrl_data,
_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, ,
_timings[ram_type_index].ddr3_data,
_cmd_ctrl_data,
_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?
Yes. If you are interested, you can see their implementation here [1],
where get_minimal_timings() returns the configuration for 1GiB;
everything is in that file. (I did credit the author.)
[1]
https://git.pengutronix.de/cgit/barebox/tree/arch/arm/boards/phytec-som-am335x/lowlevel.c#n167
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?
Agreed, the device tree solution is inferior to autodetection. The SOMs
manual makes no mention of how different variants can be
distinguished/detected, and the board specific code in Barebox (written
by Phytec) does not contain any other detection code (except for the
RAM), like checking GPIOs. Unfortunately there is no publicly available
schematic, so I can't be completely sure. So I am going to assume it,
there is no other way of detection.
Best regards,
Niel Fourie
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-21 Fax: +49-8142-66989-80 Email: lu...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
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@4a10
>
> 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
> [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, ,
> >> _timings[ram_type_index].ddr3_data,
> >> _cmd_ctrl_data,
> >> _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, ,
> >> _timings[ram_type_index].ddr3_data,
> >> _cmd_ctrl_data,
> >> _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
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
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@4a10
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
[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, ,
_timings[ram_type_index].ddr3_data,
_cmd_ctrl_data,
_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, ,
_timings[ram_type_index].ddr3_data,
_cmd_ctrl_data,
_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.
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).
Best regards,
Niel Fourie
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-21 Fax: +49-8142-66989-80 Email: lu...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
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@4a10
> >>
> >>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
> >>+void sdram_init(void)
> >>+{
> >>+ int ram_type_index = PHYCORE_R2_MT41K128M16JT_256MB;
> >>+
> >>+ if (fdtdec_setup_mem_size_base())
> >>+ gd->ram_size = SZ_256M;
> >>+
> >>+ 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, ,
> >>+ _timings[ram_type_index].ddr3_data,
> >>+ _cmd_ctrl_data,
> >>+ _timings[ram_type_index].ddr3_emif_reg_data, 0);
> >>+}
> >
> >This is wrong. sdram_init() is called by
> >arch/arm/mach-omap2/am33xx/board.c::dram_init() which then sets
> >gd->ram_size based on what get_ram_size() determines. So this is all
> >just a wrapper around how the various parts of the am33xx generations
> >call some form of config_ddr(). And what you have here is a lot of
> >unused code about which module provides how much memory. I assume
> >there's some run-time method to determine which module you're on and
> >thus determine that correct parameters to pass in for the chip that's in
> >use. If you're not there yet then just make sdram_init() call
> >config_ddr(...) with the correct enum for the 256M chip and then update
> >this when you have real detection.
>
> Thanks for that input, you are right. I could not find any documented way to
> detect the exact module we are running on, but as you pointed out we can use
> get_ram_size() to find the size of the installed RAM. This is in fact
> exactly what barebox did, I just missed it. How is this for a replacement of
> the above?
>
> 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, ,
> _timings[ram_type_index].ddr3_data,
> _cmd_ctrl_data,
> _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, ,
> _timings[ram_type_index].ddr3_data,
> _cmd_ctrl_data,
> _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?
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
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@4a10
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
+void sdram_init(void)
+{
+ int ram_type_index = PHYCORE_R2_MT41K128M16JT_256MB;
+
+ if (fdtdec_setup_mem_size_base())
+ gd->ram_size = SZ_256M;
+
+ 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, ,
+ _timings[ram_type_index].ddr3_data,
+ _cmd_ctrl_data,
+ _timings[ram_type_index].ddr3_emif_reg_data, 0);
+}
This is wrong. sdram_init() is called by
arch/arm/mach-omap2/am33xx/board.c::dram_init() which then sets
gd->ram_size based on what get_ram_size() determines. So this is all
just a wrapper around how the various parts of the am33xx generations
call some form of config_ddr(). And what you have here is a lot of
unused code about which module provides how much memory. I assume
there's some run-time method to determine which module you're on and
thus determine that correct parameters to pass in for the chip that's in
use. If you're not there yet then just make sdram_init() call
config_ddr(...) with the correct enum for the 256M chip and then update
this when you have real detection.
Thanks for that input, you are right. I could not find any documented
way to detect the exact module we are running on, but as you pointed out
we can use get_ram_size() to find the size of the installed RAM. This is
in fact exactly what barebox did, I just missed it. How is this for a
replacement of the above?
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, ,
_timings[ram_type_index].ddr3_data,
_cmd_ctrl_data,
_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, ,
_timings[ram_type_index].ddr3_data,
_cmd_ctrl_data,
_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.
Best regards,
Niel Fourie
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-21 Fax: +49-8142-66989-80 Email: lu...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
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@4a10
>
> 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
[snip]
> + memory@8000 {
> + u-boot,dm-pre-reloc;
> + };
You don't need this because:
[snip]
> +void sdram_init(void)
> +{
> + int ram_type_index = PHYCORE_R2_MT41K128M16JT_256MB;
> +
> + if (fdtdec_setup_mem_size_base())
> + gd->ram_size = SZ_256M;
> +
> + 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, ,
> +_timings[ram_type_index].ddr3_data,
> +_cmd_ctrl_data,
> +_timings[ram_type_index].ddr3_emif_reg_data, 0);
> +}
This is wrong. sdram_init() is called by
arch/arm/mach-omap2/am33xx/board.c::dram_init() which then sets
gd->ram_size based on what get_ram_size() determines. So this is all
just a wrapper around how the various parts of the am33xx generations
call some form of config_ddr(). And what you have here is a lot of
unused code about which module provides how much memory. I assume
there's some run-time method to determine which module you're on and
thus determine that correct parameters to pass in for the chip that's in
use. If you're not there yet then just make sdram_init() call
config_ddr(...) with the correct enum for the 256M chip and then update
this when you have real detection.
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support
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@4a10
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
---
Changes for v2:
- Remove formatting changes to upstream Linux dtsi files
- Remove incorrectly added MACH_TYPE
- Rename board from phycore_pcl060 to phycore_am335x_r2
- Implement selecting memory size from device tree
- Remove non-DM Ethernet board code
- General clean-up
Changes for v3:
- Added kernel revision of upstream Linux dtsi files
- Place TARGET_PHYCORE_AM335X_R2 alphabetically in Kconfig
- Rework TPS65910 I2C init code
- Remove non-DM USB board macros
- Minor tweaks
Changes for v4:
- Propose abstracting common sections to am33xx-u-boot.dtsi
- Remove dead Falcon mode code
- Remove non-DM I2C support for TPS65910 I2C
Changes for v5:
- Revert proposed moving of sections to am33xx-u-boot.dtsi
- Remove redundant/incorrect lines from am33xx-u-boot.dtsi
- Add missing changelog
arch/arm/dts/Makefile | 3 +-
arch/arm/dts/am335x-phycore-som.dtsi | 322 +
arch/arm/dts/am335x-wega-rdk-u-boot.dtsi | 31 ++
arch/arm/dts/am335x-wega-rdk.dts | 23 ++
arch/arm/dts/am335x-wega.dtsi | 230 +++
arch/arm/mach-omap2/Kconfig| 1 +
arch/arm/mach-omap2/am33xx/Kconfig | 7 +
board/phytec/phycore_am335x_r2/Kconfig | 15 +
board/phytec/phycore_am335x_r2/MAINTAINERS | 7 +
board/phytec/phycore_am335x_r2/Makefile| 11 +
board/phytec/phycore_am335x_r2/board.c | 252
board/phytec/phycore_am335x_r2/board.h | 24 ++
board/phytec/phycore_am335x_r2/mux.c | 117
configs/phycore-am335x-r2-wega_defconfig | 79 +
include/configs/phycore_am335x_r2.h| 130 +
15 files changed, 1251 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/dts/am335x-phycore-som.dtsi
create mode 100644 arch/arm/dts/am335x-wega-rdk-u-boot.dtsi
create mode 100644 arch/arm/dts/am335x-wega-rdk.dts
create mode 100644 arch/arm/dts/am335x-wega.dtsi
create mode 100644 board/phytec/phycore_am335x_r2/Kconfig
create mode 100644 board/phytec/phycore_am335x_r2/MAINTAINERS
create mode 100644 board/phytec/phycore_am335x_r2/Makefile
create mode 100644 board/phytec/phycore_am335x_r2/board.c
create mode 100644 board/phytec/phycore_am335x_r2/board.h
create mode 100644 board/phytec/phycore_am335x_r2/mux.c
create mode 100644 configs/phycore-am335x-r2-wega_defconfig
create mode 100644 include/configs/phycore_am335x_r2.h
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 8e082f2840..381a64ae13 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -264,7 +264,8 @@ dtb-$(CONFIG_AM33XX) += \
am335x-chiliboard.dtb \
am335x-sl50.dtb \
am335x-base0033.dtb \
- am335x-guardian.dtb
+ am335x-guardian.dtb \
+ am335x-wega-rdk.dtb
dtb-$(CONFIG_AM43XX) += am437x-gp-evm.dtb am437x-sk-evm.dtb\
am43x-epos-evm.dtb \
am437x-idk-evm.dtb \
diff --git a/arch/arm/dts/am335x-phycore-som.dtsi
b/arch/arm/dts/am335x-phycore-som.dtsi
new file mode 100644
index 00..8d7c19e5e1
--- /dev/null
+++ b/arch/arm/dts/am335x-phycore-som.dtsi
@@ -0,0 +1,322 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2015 Phytec Messtechnik GmbH
+ * Author: Teresa Remmet
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "am33xx.dtsi"
+#include
+
+/ {
+ model = "Phytec AM335x phyCORE";
+ compatible = "phytec,am335x-phycore-som", "ti,am33xx";
+
+ aliases {
+ rtc0 = _rtc;
+ rtc1 =
+ };
+
+ cpus {
+ cpu@0 {
+ cpu0-supply = <_reg>;
+ };
+ };
+
+ memory@8000 {
+ device_type = "memory";
+ reg = <0x8000 0x1000>; /* 256 MB */
+ };
+
+ regulators {
+ compatible = "simple-bus";
+
+ vcc5v: fixedregulator0 {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc5v";
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ };
+};
+
+/* Crypto Module */
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+/* Ethernet */
+_pinmux {
+ ethernet0_pins: pinmux_ethernet0 {
+ pinctrl-single,pins = <
+
