RE: [PATCH v2] dt-bindings: interrupt-controller: aspeed: Add parent node compatibles and refine documentation

Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: aspeed: Add parent
node compatibles and refine documentation

On Tue, Jul 15, 2025 at 10:42:58AM +0800, Ryan Chen wrote:

quoted

- Add 'aspeed,ast2700-intc0' and 'aspeed,ast2700-intc1' compatible
strings for parent interrupt controller nodes, in addition to the
existing 'aspeed,ast2700-intc-ic' for child nodes.
- Clarify the relationship and function of INTC0, INTC1, and the GIC.
- Update and clarify documentation, block diagram, and examples to
reflect the hierarchy and compatible usage.
- Documentation and example refine.

So 7 lines describing obvious - what you did and three lines below describing
non-obvious, why you did it. It should be reversed.

Thanks your feedback.

How about following description?

The AST2700 SoC contains two independent top-level interrupt controllers (INTC0 and INTC1),
each responsible for handling different peripheral groups and occupying separate register spaces.
Above them, a GIC controller acts as the global interrupt aggregator. 
Accurately describing this hierarchical hardware structure in the device tree requires distinct compatible strings for the parent nodes of INTC0 and INTC1.

- Adds 'aspeed,ast2700-intc0' and 'aspeed,ast2700-intc1' compatible strings for parent interrupt controller nodes (in addition to the existing 'aspeed,ast2700-intc-ic' for child nodes)
- Clarifies the relationship and function of INTC0 parent (intc0_0~x: child), INTC1 parent (intc1_0~x: child), and the GIC in the documentation
- Updates block diagrams and device tree examples to illustrate the hierarchy and compatible usage
- Refines documentation and example formatting

quoted

This change allows the device tree and driver to distinguish between

Why driver needs would matter here?

quoted

parent (top-level) and child (group) interrupt controller nodes,
enabling more precise driver matching SOC register space allocation.

This just does not make sense. You do not change "precise driver matching" via
bindings. You fix driver. Especially that there is no driver patch here at all and
aspeed,ast2700-intc0 are totally unused!
Don't add ABI which has no users.

Again, you need to start describing the hardware and the REASONS BEHIND
from the hardware point of view. Not drivers.

This change alone based on above explanation makes no sense at all.

Next version, I will move the addition of aspeed,ast2700-intc0 and aspeed,ast2700-intc1 compatible strings into the driver patch,
so they are added together with actual driver support.

Thank you for your guidance.

Best regards,
Krzysztof