Re: [PATCH 0/3] dt-bindings: arm: qcom: define schema, not devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 25 Jul 2022 at 19:18, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 24/07/2022 00:50, Dmitry Baryshkov 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.
>
> Schema is a tool, we create here bindings. The bindings document what
> you wrote above, plus compatibles and any other pieces mentioned in DT spec.
>
> > 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.
>
> Although one of goals is to have schema compliance, that is not the
> reason why we document compatibles. Compatibles were documented long
> time ago before DT schema came, because the bindings require it.
>
> Bindings define the interface between the DTS and software which uses
> it. SW is kernel, bootloaders, user-space and some more.

I completely agree here.

>From the point of HW/SW interfaces maybe the following compat lists
would make more sense:
- "xiaomi,msm8996-phone", "qcom,msm8996"
- "qcom,qrb5165-iot-platform", "qcom,sm8250"
- "qcom,sda845-iot-platform", "qcom,sdm845"
- "inforce,sda660-sbc", "qcom,sda660"

etc. IOW, describing the kind of the device, not the exact name/model of it.

>
> > For each and every phone/sbc/evb we have to name it in schema. I think
> > this is an overkill.
>
> Qualcomm is rather moderate, nothing big here so definitely it is not an
> overkill. We almost do not have there SoMs. Just take a pick at
> Freescale - this is much more complex than Qualcomm, so any changes
> should start with that. Qualcomm's "complexity" is not a reason to do
> anything...

Yeah, been there. As I wrote earlier, this looks like a postfixItems use case.

>
> > It feels like we are putting DT information
> > (mapping form machine compat to SoC) into the DT schema.
>
> No, we are documenting the compatible in bindings. Just like we always
> did and we always had to.
>
> > 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.
> >
> > 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.
>
> Again, Qualcomm complexity is nothing compared to Freescale. :)

Fortunately :-)
I was trying to propose a way to have a useful schema, while keeping
it small enough.

> > 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".
> >
> > 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.
>
>
> No, because you miss then the purpose of bindings - to document the
> compatible which is the important piece of interface between
> DTS/bootloader/kernel/other OS/user-space.
>
> To summarize, you propose to not document board compatibles. This is not
> what we want, because then the next step is - let's don't document SoCs.
> If you do not document it, means anyone can uniliteraly change it, e.g.
> in kernel DTS, which will break all other users (e.g. user-space or
> bootloaders) parsing the compatibles. And before you say - no one parses
> the board compatibles - let me just say that several user-space
> (embedded/closed) parse them, bootloaders parse them (U-Boot, Google's
> Chromebooks and others)

Yes, I know. And in the case of e.g. Google ChromeOS bootloader it
might make sense to declare compatibles using patterns rather than
enumeration.

Anyway, thank you for your comments, let's continue with documenting
all the devices until something changes.

-- 
With best wishes
Dmitry



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux