Re: [RFC PATCH 2/6] dt-bindings: bus: Add qcom,soc-sc7180 SoC

Rob Herring
2025-01-10T13:58:44Z

On Thu, Jan 09, 2025 at 01:51:12PM -0800, Stephen Boyd wrote:
> Quoting Konrad Dybcio (2025-01-09 06:05:14)
> > On 8.01.2025 2:28 AM, Stephen Boyd wrote:
> > > Document the Qualcomm SC7180 System on a Chip (SoC). This SoC is made up
> > > of multiple devices that have their own bindings, therefore this binding
> > > is for a "bus" that is the SoC node.
> > >
> > > TODO: Document all child nodes. This is woefully incomplete but at least
> > > shows what is involved with describing an SoC node in dt schema.
> >
> > I'm not sure I'm a fan, because...
> >
> > [...]
> >
> > > +
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - const: qcom,soc-sc7180
> > > + - const: simple-bus
> > > +
> > > + '#address-cells':
> > > + const: 2
> > > +
> > > + '#size-cells':
> > > + const: 2
> > > +
> > > + clock-controller@100000:
> > > + $ref: /schemas/clock/qcom,gcc-sc7180.yaml#

This makes the above schema be applied twice. Once here and then when
the compatible matches. That can be avoided by just listing a
compatible. The QCom display bindings follow that style.

> > > +
> > > + watchdog@17c10000:
> > > + $ref: /schemas/watchdog/qcom-wdt.yaml#
> > > +
> > > +required:
> > > + - compatible
> > > + - '#address-cells'
> > > + - '#size-cells'
> > > + - clock-controller@100000
> > > + - watchdog@17c10000
> > > +
> > > +additionalProperties: false
> >
> > ..this approach will make any dt node addition under /soc require
> > an additional bindings change, which sounds like absolute madness
>
> We should pretty much know what nodes go under here though, because it's
> a chip that exists and doesn't change after the fact. I agree that it
> will be annoying during early development when everyone is modifying the
> same file to add their node, but that problem also exists with the dts
> files today so it doesn't seem like total madness. It's also nice to be
> able to look at one file and quickly find all the schemas for the SoC
> used, like a table of contents almost or a memory map for the chip.

I don't really care for listing everything either.

We could just generate all the schemas used. Either "give me all the
schemas matching some compatible patter" or "give me all the schemas
used to validate the DTB". The latter was possible on a per node basis,
but I think I dropped that when I changed how we select schemas to
apply.

Speaking of memory maps, I would like a tool which could dump memory map
from .dts. My primary reason is to find overlapping regions.

> One thing that I find annoying is that you have to put the whole soc
> node and child nodes in the example. Maybe we can omit the example
> because there are so many child nodes.
>
> >
> > I think additionalProperties: true would be sufficient here, like in
> > Documentation/devicetree/bindings/soc/imx/imx8m-soc.yaml
> >

No. You can do:

additionalProperties:
type: object

Or a patternProperties entry requiring '@' in the name.

> Ok. That binding looks to be for the efuse properties of the SoC node
> itself? I was hoping to find another example of this "describe the whole
> SoC" sort of binding but that doesn't match. Is there one already out
> there? Should I move this binding to bindings/soc/qcom instead of
> bindings/bus?

bindings/bus

The 'soc' nodes here aren't really for the whole SoC. Cpus aren't in
the soc node. They are for buses. We should allow for there to be more
than 1 for instance.

Rob