On Monday 16 January 2023 16:33:09 Tony Dinh wrote:
Hi all,
On Sat, Jan 14, 2023 at 1:45 PM Tony Dinh mibodhi@gmail.com wrote:
Hi Pali & Tom,
On Sat, Jan 14, 2023 at 12:06 PM Pali Rohár pali@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?
Thanks, Tony
Well, the best is to not change licence headers in existing u-boot files.