Hi,
On Sat, Jul 23, 2022 at 3:51 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> On Sat, 23 Jul 2022 at 20:48, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >
> > On 23/07/2022 11:09, Dmitry Baryshkov wrote:
> > > Describing each compatible board in DT schema seems wrong to me. It
> > > means that each new board is incompatible by default, until added to the DT
> > > schema.
>
> s/incompatible/non-valid/
>
> >
> > The bindings do not document something because it is or it is no
> > compatible. They document the compatible. Your patch essentially removes
> > the documentation, so there is no point in having compatibles in board
> > at all...
>
> I do not quite agree here. Please correct me if I'm wrong.
> Schema defines which DT files are correct and which are not. Which
> properties are required, which values are valid, etc. So far so good.
> For the device nodes it declares (or is willing to declare) all
> possible compatibility strings. There is a sensible number of on-chip
> IP blocks, external chips, etc. Each and every new chip (or new IP
> block) we are going to have a yaml file describing its usage. Perfect.
> For the machine compatibility lists the arm,qcom schema declares which
> machine compat strings are valid. And this looks like a problem. Now
> for the DT to be fully valid against DT schema, we have to define its
> machine compat string.
> For each and every phone/sbc/evb we have to name it in schema. I think
> this is an overkill. It feels like we are putting DT information
> (mapping form machine compat to SoC) into the DT schema.
> For qcs404 we already have a schema which uses three items: {},
> qcom,qcs404-evb, qcom,qcs404. This sounds like a perfect idea to me.
> We allow any board, created by Qualcomm, Google or any other random
> vendor, named Foo, Bar or Baz, as long as it declares that it is
> compatible with qcom,qcs404-evb.
I assume the above was supposed to be "as long as it declares that it
is compatible with qcom-qcs404" since everywhere else you're declaring
that only the SoC compatible is important.
In any case, I would tend to agree that the most useful thing that the
schema here is doing is validating that we didn't have a typo in the
SoC field. I already tried to argue that adding every board into this
yaml file seemed like extra overhead without a lot of benefit but I
finally ceded to do the busy work instead of continuing to argue.
As a side note, at one point Stephen Boyd was on a quest to get the
"SoC" removed from the top-level compatible and moved to the "SoC"
node. I dunno if that's still something he's pursuing.
> To go even further. We now have the qrb5165-rb5.dts, declaring
> compatible = "qcom,qrb5165-rb5", "qcom,sm8250".
> If at some point I add a navigation/communication/whatever mezz on top
> of it. It would make sense to use compatible =
> "qcom,qrb5165-rb5-navi-comm", "qcom,qrb5165-rb5", "qcom,sm8250".
> Adding this to the growing list inside arm,qcom.yaml sounds like a
> nightmare. I can only hope that at some point JSON schema gains
> postfixItems support (as proposed) to be able to change e.g.
> arm,qcom.yaml to list just our qcom,something platforms as possible
> postfixItems for the schema.
>
> Regarding having compatibles in board files at all. I think that they
> are somehow misused nowadays. Instead of declaring that the
> Dragonboard 845c is compatible with "thundercomm,db845c",
> "qcom,sdm845-sbc", "96boards,ce-board", "96boards,iot-board",
> "qcom,sdm845", the DT file declares nearly useless
> "thundercomm,db845c", "qcom,sdm845".
Again a little bit of an aside, but one point I've tried
(unsuccessfully) to make in the past is that, at least for the
ChromeOS bootloader (and presumably anyone else using a FIT image),
the top-level compatible works almost completely opposite of all the
rest of the compatibles.
For most compatible strings, you start with a known device tree file
which declares the compatibles for a peripheral. You then search for a
driver that can handle that peripheral. You start with the first
string and try to find a driver for that. If you can't, you go onto
the second string which is supposed to work, just not as well. Great.
The top level compatible string, at least in the case of ChromeOS, is
used by the bootloader to pick among all the device tree files it has.
That's a critical difference.
Let me try to be concrete. Let's say you have two boards that are
exactly identical but one has LTE components stuffed and one doesn't
(it's WiFi only). It's fine to treat the LTE board as a WiFi-only
board but not OK to treat the WiFi-only board as an LTE board (it'll
crash). The bootloader knows (through its own means) whether you're on
LTE or WiFi-only and is given a pile of device trees.
Let's look specifically at the device tree file for the LTE board. One
way to look at it is that the dts for the LTE board should have
compatibles:
compatible = "lte", "wifi-only"
The above matches the normal device tree mentality. It says: "hey, if
you've got a lte driver for this board then use it; otherwise use the
wifi-only driver".
However, the above is actually broken for the bootloader use case. The
bootloader is trying to pick a device tree and, to the bootloader, the
above says "you can use this dts for either an lte board or a
wifi-only board". That's bad. If the bootloader picks this device tree
for a wifi-only board then the OS will try to initialize lte and
things will crash. To go further, if you think about it things
actually work fine if the wifi-only device tree says it's compatible
with the LTE board. This is why I say it's opposite... ;-)
> Thus, if we (mostly) use machine compatible array to just define
> Vendor+device name and the underlying Qualcomm SoC, I propose to leave
> just a sensible part (SoC) in DT schema, while allowing any string in
> the first part.
>
> > >Adding support for more and more devices would grow this file
> > > indefinitely. Drop most of individual device-specific compatibility
> > > strings leaving just list of platforms in place. All entries which
> > > differ from two-item string array are left inplace.
>
> --
> With best wishes
> Dmitry
