Re: [PATCH 04/10] dt-bindings: interrupt-controller: Add i.MX8qxp Display Controller interrupt controller

Krzysztof Kozlowski
2024-07-08T14:09:42Z

On 08/07/2024 08:51, Liu Ying wrote:
> On 07/07/2024, Krzysztof Kozlowski wrote:
>> On 05/07/2024 11:09, Liu Ying wrote:
>>> i.MX8qxp Display Controller has a built-in interrupt controller to support
>>> Enable/Status/Preset/Clear interrupt bit.
>>>
>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>> ---
>>> .../fsl,imx8qxp-dc-intc.yaml | 321 ++++++++++++++++++
>>> 1 file changed, 321 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,imx8qxp-dc-intc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,imx8qxp-dc-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,imx8qxp-dc-intc.yaml
>>> new file mode 100644
>>> index 000000000000..3d0d11def505
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,imx8qxp-dc-intc.yaml
>>> @@ -0,0 +1,321 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/interrupt-controller/fsl,imx8qxp-dc-intc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Freescale i.MX8qxp Display Controller interrupt controller
>>> +
>>> +description: |
>>> + The Display Controller has a built-in interrupt controller with the following
>>> + features for all relevant HW events:
>>> +
>>> + * Enable bit (mask)
>>> + * Status bit (set by an HW event)
>>> + * Preset bit (can be used by SW to set status)
>>> + * Clear bit (used by SW to reset the status)
>>
>> 1. Why this is split from the main node?
>
> Maxime suggested to do so:
>
> "
> But at least the CRTC and the interrupt controller should be split away,
> or explained and detailed far better than "well it's just convenient".
> "
>
> https://lore.kernel.org/lkml/2k3cc3yfwqlpquxrdmzmaafz55b3lnqomzxjsvtetfriliqj3k@tv6uh7dzc2ea/
>
>>
>> 2. Who can use this interrupt controller? Children of your display unit?
>
> Yes, only devices in the main display controller use it.
>
>> Then it is not really a separate device, is it?
>
> Er, per Maxime, it is a separate device.

It's not for me, especially considering small register region used here.

Srsly, with that claim, some I2C device like PMIC also has a separate
interrupt controller, because interrupts are in separate few registers
(e.g. status, mask and ack).

Can any of the children interrupts be routed differently?

Post entire, complete DTS for review.

Best regards,
Krzysztof