On Sun, May 07, 2023 at 06:06:40PM +0200, Marek Vasut wrote:
On 4/24/23 03:15, Ralph Siemsen wrote:
Add support for Schneider Electronics RZ/N1D and RZ/N1S boards, which
are based on the Reneasas RZ/N1 SoC devices.
The intention is to support both boards using a single defconfig, and to
handle the differences at runtime.
The DT comes from Linux kernel, right ? Please include commit ID from
which the DT is imported in the commit message, I suspect that would
be Linux 6.3 commit ID.
Yes, the DT is copied verbatim from Linux. I have updated the commit
message to mention 6.3 and include the hash. (There have been no changes
diff --git a/board/schneider/rzn1-snarc/ddr_async.c
b/board/schneider/rzn1-snarc/ddr_async.c
new file mode 100644
index 0000000000..4b4c280e45
--- /dev/null
+++ b/board/schneider/rzn1-snarc/ddr_async.c
Please correct me if I'm wrong, but shouldn't this be in drivers/ram/ ?
I had it there originally, but moved it in v5 of the patch series. There
is a lot of board-specific (or at least architecture-specific) logic in
this file. For example the sequence of steps to setup the clocks and
interconnect, prior to bringing the DDR controller out of reset. This
depends on choices made by the RZ/N1 designers, rather than the CDNS IP
that drivers/ram/cadence is aiming to cover.
Likewise for some board-specific PHY settings, and choices such as
operating in async mode.
I moved it to board-specific directory as an interim step. Hopefully we
can do some consolidation of the multiple CDNS DDR controller
implementations, and then figure out the right way to split things up.
For now this seemed like the less-bad option.
+
+ /* Step 15 Wait for 200us or more, or wait for DFIINITCOMPLETE to be
"1" */
+ while (!(phy_readl(DLLCTRL) & DLLCTRL_ASDLLOCK))
+ ;
Please avoid endless loops, use readl_poll_timeout() or
wait_for_bit*() where possible.
Will do.
+int board_init(void)
+{
+ /*
+ * Initial values for gd->ram_base and gd->ram_size
+ * are obtained from the "/memory" node in devicetree.
+ * The size will be updated in later when probing DDR.
+ */
+ fdtdec_setup_mem_size_base();
Are you absolutely sure this call ^ is needed at all ?
I was sure at the time when I wrote it, however I just tried removing it
now and it seems to work fine.
You can just do this here:
err = uclass...();
if (err)
debug(...);
return err;
Done.
+#ifndef __RZN1_H
Better use __RZN1_SNARC_H , so that when other boards get added, there
won't be trouble.
Good catch, done.
Ralph
