Hi Krzysztof, krzysztof.kozlowski@xxxxxxxxxx wrote on Fri, 16 Jun 2023 10:15:31 +0200: > On 15/06/2023 23:06, Chris Packham wrote: > >> > >>> + > >>> +properties: > >>> + compatible: > >>> + oneOf: > >>> + - items: > >>> + - const: marvell,armada-8k-nand-controller > >>> + - const: marvell,armada370-nand-controller > >>> + - enum: > >>> + - marvell,armada-8k-nand-controller > >> This is wrong. 8k cannot be both: compatible and not compatible with > >> 370. It's not someone's cat to be in both states at the same time... > > > > The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible > > = "marvell,armada-8k-nand-controller";` as there are some 8K specific > > requirements that aren't present on the 370 (specifically the > > system-controller and the 2nd clock). > > Presence of specific requirements does not invalidate compatibility. Two > devices are compatible if the 8k can bind and work with 370 compatible > string, even if this means some lower performance or less features (e.g. > subset of features). Quoting myself from 2019 (comment in the driver): "Some SoCs like A7k/A8k need to enable manually the NAND controller, gated clocks and reset bits to avoid being bootloader dependent." So can the 8k controller work using a 370 compatible : yes, if the booloader enabled the NAND controller already, no otherwise. But in practice it is the same controller. Given this information I don't know whether it makes sense to qualify the 8k controller compatible with the 370 compatible or not. > Now whether they are really compatible or not - I don't know. I expect > the answer from you (as plural you). :-) > > The only reason `compatible = > > "marvell,armada-8k-nand-controller", > > "marvell,armada370-nand-controller";` is supported is because that is > > what is in use in the existing dtses. > > Fix the DTS, use here the correct combination and, if it is a deviation > from old binding, mention this in commit msg. Fine by me. > Best regards, > Krzysztof > Thanks, Miquèl
