Re: [PATCH v4 2/7] nvme: assign of_node to nvme device

[Christian Marangi <ansuelsmth@gmail.com>]

On Mon, Aug 12, 2024 at 01:12:05PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 09, 2024 at 07:21:00PM +0200, Christian Marangi wrote:
> > Introduce support for a dedicated node for a nvme card. This will be a
> > subnode of the nvme controller node that will have the "nvme-card"
> > compatible.
> 
> FYI, there really is no such thing as an NVMe card.  There is an
> NVMe Namespace, which is the entity that contains the block data,
> the Controller which corresponds to the pci_dev for NVMe-PCIe, and
> the NVMe Subsystem, which contains Controllers and Namespaces.
>

The chosen name was arbritrary just to follow eMMC ones. Can totally
change if problematic.

> > This follow a similar implementation done for mmc where the specific mmc
> > card have a dedicated of_node.
> 
> That's not a good explanation to be honest.  Most eMMC host controllers
> are OF probed devices, so of course they'll have an of_node.
> 
> Binding PCIe functions to of_nodes seems completely weird to me, and
> you'll need to explain what this totally non-obvious thing makes sense.
> Maybe it does, but it needs to be backed up with a very good rationale
> that is very clearly documented.
> 

But support of OF for PCIe is already a thing for a long time. (it all
works by setting the compatible of the PCIe ID card) and used in wifi
card at assign MAC address, calibration data, disable frequency.

In this context we would do a similar thing with declaring the NVMe with
the PCIe ID card (already supported) and we add support for defining an
additional subnode for usage of block2mtd.

The subnode is needed to keep consistency in how the disk struct are
defined with all the parenting levels.

Just to stress it more... This is really for consistency as PCIe OF node
are already a thing and on block2mtd (for example) the same thing can be
done with something like

disk->parent->parent.of_node (as it would point, if present, to the OF node
of the PCIe card (the NVMe))

With eMMC with the mmc-card subnode we would have to use

disk->parent.of_node

Not having this well organized and consistent schema in DT will result
in additional condition in the drivers...

Also consider that if the card is not detected, nothing is probed so
those additional node won't cause any harm.

If these 2 patch are problematic I can totally drop from the series but
it was really to add consistency in NVMe and eMMC. The real important
part is eMMC that is becoming the de-facto replacement for NAND/NOR on
high tier devices (mostly wifi6/7 consumer router)

-- 
	Ansuel