Re: [PATCH v1 00/23] board: nxp: add the Free Mobile Nodebox v3 CPU Module (LX2160A)

Hi Vincent,

> > > This series adds the Free Mobile Nodebox v3 CPU Module, an NXP
> > > LX2160A based board (16x Cortex-A72, DPAA2). U-Boot is the BL33
> > > payload of TF-A: BL2 trains the DDR, BL31 hands over, and U-Boot
> > > boots a FIT image (kernel + MC firmware + DPC/DPL) from a single
> > > FlexSPI NOR. The module plugs onto several carrier boards (NBV30,
> > > NBV32, etc.) that share one kernel image but differ in their device
> > > tree and DPAA2 data path.
> >
> > Why just FIT and not UEFI?
>
> Because this layerscape platform shall not support UEFI by design.

Really? I have a clearfog LX2 platform that boots and runs with UEFI
just fine. Generally this is where the Linux ecosystem is going so for
a new board going upstream I would expect it to at least be an option.

> > > The port is modelled on the LX2160ARDB / QDS boards and lives next
> > > to them under board/nxp/lx2160a/, reusing their SoC level code.
> > > Board specific behaviour is wired through EVENT_SPY hooks rather
> > > than weak symbol overrides, and the dependencies each hook needs
> > > are forced on by hidden default-y Kconfig bools under
> > > `if TARGET_NBXV3`, so a hook cannot silently turn into a no-op.
> > >
> > > The series is in three stages:
> > >
> > >  1. Base port and documentation (patches 1-2): TARGET_NBXV3, the
> > >     defconfig, a minimal device tree, the MAINTAINERS entry, the
> > >     Ethernet wiring file and doc/board/nxp/nbxv3.rst.
> > >
> > >  2. Board code (patches 3-9): DPAA2 Ethernet on DPMAC17 through the
> > >     RTL8211FD PHY; the +0V8_VDD core rail (MPS MPQ8785 PMBus
> > >     regulator snapshot, PMBus PSU snapshot, VID fuse driven voltage
> > >     trim through UCLASS_REGULATOR); the ${carrier} selection that
> > >     lets one FIT carry a DPC/DPL/DTB set per carrier; the ZL30733 /
> > >     ZL30643 DPLL traces; the +3V3_FAN rail hook.
> > >
> > >  3. Build and device tree settings (patches 10-23): one small
> > >     commit per peripheral or features, each enabling its DT nodes and
> > >     defconfig symbols.
> >
> > The device tree should be sent upstream to the Linux kernel and then I
> > think most of 10-23 should be compacted to a single patch.
>
> I get your point of Linux first and uboot policy, however, this board
> follow the legacy support of other NXP's lx2160/layerscape boards. For
> example,
>
>  arch/arm/dts/fsl-lx2160a-rdb.dts
>  arch/arm/dts/fsl-lx2160a-qds.dts
>  arch/arm/dts/fsl-lx2162a-qds.dts

There's a number of fsl-lx216x boards with upstream linux DTs.

> and LX2160A defconfigs currently use CONFIG_DEFAULT_DEVICE_TREE without
> CONFIG_OF_UPSTREAM.

But those boards still have upstream device trees.

> Moreover, per https://docs.u-boot.org/en/latest/develop/devicetree/control.html,
> "Configuration" section: it says that SoC/board maintainers are encouraged to
> migrate to dts/upstream/, but also says that when dts/upstream/ has not yet
> received the device tree for newly added board support, one option is to add it as:
>   arch/<arch>/dts/<name>.dts
> with CONFIG_OF_UPSTREAM disabled.

Even boards that don't use OF_UPSTREAM because of outlying deltas
still typically have their DTs upstream and there's syncs between
them. see the lists archives over the years for numerous examples.

So yes, board maintainers are encouraged to migrate to upstream, but
you've missed my point generally. Boards are generally required to
have an upstream DT even if in U-Boot they don't use the OF_UPSTREAM
infrastructure. That is to ensure that the DT has some level of review
and sanity and isn't just thrown over the fence. Hence why I asked.

> So how would it be acceptable to have fsl-lx2160a-nbxv3.dts under arch/arm/dts/,
> consistently with the existing LX2160A boards and the option described in the
> U-Boot documentation?

No, please get review of the DT upstream even if you end up putting it
in arch/arm/dts/ to ensure we end up with something that is
maintainable.

> Should those patches 10-23 be 1 commit, I can squash them too: I wanted to record
> the logics of each feature sets.
>
> Best regards,
>   Vincent