[PATCH] ddr: marvell: a38x: Add support for DDR4 from Marvell mv-ddr-marvell repository

[Tony Dinh]

On Mon, Jan 16, 2023 at 4:33 PM Tony Dinh <mibodhi at gmail.com> wrote:
>
> Hi all,
>
> On Sat, Jan 14, 2023 at 1:45 PM Tony Dinh <mibodhi at gmail.com> wrote:
> >
> > Hi Pali & Tom,
> >
> > On Sat, Jan 14, 2023 at 12:06 PM Pali Rohár <pali at kernel.org> wrote:
> > >
> > > On Saturday 14 January 2023 15:03:41 Tom Rini wrote:
> > > > On Sat, Jan 14, 2023 at 07:51:00PM +0100, Pali Rohár wrote:
> > > > > On Friday 13 January 2023 21:00:21 Tom Rini wrote:
> > > > > > On Sat, Jan 14, 2023 at 02:41:32AM +0100, Pali Rohár wrote:
> > > > > > > On Friday 13 January 2023 16:38:55 Tony Dinh wrote:
> > > > > > > > @@ -16,4 +19,9 @@ obj-$(CONFIG_SPL_BUILD) += mv_ddr_build_message.o
> > > > > > > >  obj-$(CONFIG_SPL_BUILD) += mv_ddr_common.o
> > > > > > > >  obj-$(CONFIG_SPL_BUILD) += mv_ddr_spd.o
> > > > > > > >  obj-$(CONFIG_SPL_BUILD) += mv_ddr_topology.o
> > > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_mpr_pda_if.o
> > > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training.o
> > > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_calibration.o
> > > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_db.o
> > > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_leveling.o
> > > > > > > >  obj-$(CONFIG_SPL_BUILD) += xor.o
> > > > > > >
> > > > > > > And all these new files are ddr4 specific, so should be wrapped in makefile section:
> > > > > > > ifdef CONFIG_DDR4
> > > > > >
> > > > > > Looking at the Makefile in question, I think we might want to make the
> > > > > > whole thing ifdef CONFIG_SPL_BUILD ... endif and then more finely
> > > > > > control building of what objects are built.  Perhaps:
> > > > > > drivers/Makefile:obj-$(CONFIG_ARMADA_38X) += ddr/marvell/a38x/
> > > > > > should only be for SPL instead, even?
> > > > >
> > > > > Some cleanup like this can be done. But it is related to DDR4 support
> > > > > and is mostly independent of it. So lets do it after having DDR4 there.
> > > >
> > > > We're going to also want to not build the DDR3 code on the DDR4
> > > > platforms, right? A little clean up would make adding the DDR4 code a
> > > > bit cleaner for both cases. It's not a hard no, if someone really wants
> > > > to do the clean-up after.
> > >
> > > I can look at it _after_ all other stuff is done and merged.
> >
> > Thanks for the review and comments! and thanks Pali in advance for the
> > Makefile improvement after.  I will submit the V2 patches to fix the
> > dead code and other editorial issues per Pali's review.
>
> Am I correct in assuming that the preferred license header is
> // SPDX-License-Identifier: GPL-2.0
>
> and _not_
> /* SPDX-License-Identifier: GPL-2.0 */
>
> Or does it not matter which form we use as long as it is consistent in an area?

NVM, I was scratching my head why patman kept warning me for using one
form or the others!

/Licenses/README

   The SPDX license identifier is added in the form of a comment.  The comment
   style depends on the file type::

      C source: // SPDX-License-Identifier: <SPDX License Expression>
      C header: /* SPDX-License-Identifier: <SPDX License Expression> */
      ASM:      /* SPDX-License-Identifier: <SPDX License Expression> */
      scripts:  # SPDX-License-Identifier: <SPDX License Expression>
      .rst:     .. SPDX-License-Identifier: <SPDX License Expression>
      .dts{i}:  // SPDX-License-Identifier: <SPDX License Expression>

All the best,
Tony