Re: [PATCH V3 1/2] dt-bindings: interrupt-controller: Add Loongson EIOINTC

[Binbin Zhou <zhoubb.aaron@gmail.com>]

On Thu, Apr 20, 2023 at 11:52 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/04/2023 15:00, Binbin Zhou wrote:
> >>> +examples:
> >>> +  - |
> >>> +    eiointc: interrupt-controller@1fe11600 {
> >>> +      compatible = "loongson,ls2k0500-eiointc";
> >>> +      reg = <0x1fe11600 0x10>,
> >>> +            <0x1fe11700 0x10>,
> >>> +            <0x1fe11800 0x10>,
> >>> +            <0x1fe114c0 0x4>;
> >>
> >> Binding is OK, but are you sure you want to split the address space like
> >> this? It looks like two address spaces (enable+clear+status should be
> >> one). Are you sure this is correct?
> >>
> > Hi Krzysztof:
> >
> > These registers are all in the range of chip configuration registers,
> > in the case of LS2K0500, which has a base address of 0x1fe10000.
> > However, the individual register addresses are not contiguous with
> > each other, and most are distributed across modules, so I feel that
> > they should be listed in detail as they are used.
>
> Do you want to say that:
> Between 0x1fe11600 and 0x1fe11700 there are EIOINTC registers and other
> (independent) module registers?

No, this section is all EIO-related configuration, but there will be
undefined space here.

Throughout the chip configuration space, there are some relatively
common areas, such as the definition of 0x1fe1_14c0.
Because our chip supports two interrupt modes, node legacy I/O
interrupt and extended I/O interrupt, both modes require interrupt
routing registers.
Their registers are then defined together: the legacy interrupt I/O
start address is 0x1fe1_1400, while the extended I/O interrupt start
address is 0x1fe1_14c0.

Then I have carefully compared the chip configuration space in
LS2K0500 and LS2K2000 and can see that:

1. The chip configuration space base addresses are different, but they
both have a size of 64KB;
2. The offset addresses of the EIO related registers are the same, for
example the offset of the enable register is 0x1600.

Wouldn't it be better to declare the entire configuration space (64KB)
directly in the dts and use the offsets to access the corresponding
registers?

Example:
reg = <0x1fe10000 0x10000>.

Thanks.
Binbin

>
> Best regards,
> Krzysztof