ARM: mach-shmobile: sh7372 DT IRQ prototype

From: Magnus Damm <[email protected]>

This prototype patch extends the sh7372 DT support with actual IRQ support. The heavy lifting is all done by the prototype patch
"[PATCH] sh: INTC IRQ domain and DT support prototype".

It is worth noting that we have 3 root interrupt controllers on sh7372 and to set the parent pointer to NULL we let the device tree point to itself as interrupt-parent. Seems a bit overly complex so perhaps it is not the right way forward.

All root interrupt controllers are using IRQ domains but their allocation is kept the same as the non-DT case to allow us to leave the demux code as-is without any modifications. So in the DT case INTCA, IRQ16L and IRQ16H are kept at their original IRQ positions. INTCS is allocated dynamically in case of DT and at a fixed location as usual for non-DT boards.

Anyone that wants to play with DT with interrupts on sh7372 will need to include this patch. The INTC IRQ domain patch above needs further work which will delay the merge.

Not-yet-signed-off-by Magnus Damm <[email protected]>

Quoted 22 lines · Magnus Damm

From: Magnus Damm <[email protected]>

This prototype patch extends the sh7372 DT support with actual IRQ support. The heavy lifting is all done by the prototype patch "[PATCH] sh: INTC IRQ domain and DT support prototype".

It is worth noting that we have 3 root interrupt controllers on sh7372 and to set the parent pointer to NULL we let the device tree point to itself as interrupt-parent. Seems a bit overly complex so perhaps it is not the right way forward.

All root interrupt controllers are using IRQ domains but their allocation is kept the same as the non-DT case to allow us to leave the demux code as-is without any modifications. So in the DT case INTCA, IRQ16L and IRQ16H are kept at their original IRQ positions. INTCS is allocated dynamically in case of DT and at a fixed location as usual for non-DT boards.

Anyone that wants to play with DT with interrupts on sh7372 will need to include this patch. The INTC IRQ domain patch above needs further work which will delay the merge.

Quoted 1 line

Not-yet-signed-off-by Magnus Damm <[email protected]>

Hi Magnus,

I'm trying to find my way through your patches, but I still have a little trouble figuring out how it all fits together.

My feeling is that the soc specific parts can be done better if you generalize the interrupt controller bindings so that you can describe the controller(s) as a single device, and allow all the information that is now in the intc-*.c files to be moved into device tree attributes to be parsed at boot time, or from the translate() function of the interrupt controller driver.

That feeling may of course be completely wrong. Do you have a link to a data sheet describing how that controller actually works, or can you explain what the various arrays are needed for today, and how the various interrupt controllers you register fit together?

Quoted 4 lines

Hi Magnus,

I'm trying to find my way through your patches, but I still have a little trouble figuring out how it all fits together.

Right, sorry for spitting out patches to the left and to the right. =)

So the full dependencies for "base DT support" are:

renesas.git a6e24019468009a21b674e392d74283a90f415dd (origin/master) [PATCH 00/06] mach-shmobile device tree preparation patches V2 [PATCH] ARM: mach-shmobile: sh7372 generic board support via DT V2

The patches above are rather ready IMO, and they've got Signed-off-by.

To get prototype level support of IRQs you will also need the following:
[PATCH] sh: INTC IRQ domain and DT support prototype
[PATCH] ARM: mach-shmobile: sh7372 DT IRQ prototype

The two patches above are experimental with Not-yet-signed-off-by.

There are more patches as well, but most depend on the prototype IRQ code above.

Quoted 7 lines

My feeling is that the soc specific parts can be done better if you generalize the interrupt controller bindings so that you can describe the controller(s) as a single device, and allow all the information that is now in the intc-*.c files to be moved into device tree attributes to be parsed at boot time, or from the translate() function of the interrupt controller driver.

For sh7372 and most other of our SoCs I believe it will be difficult to use a single device for interrupts. This since there are a few different interrupt controllers working together, and they have different interrupt ranges associated with them. They fit quite well with IRQ domains and the ->dt_translate() callback and gluing them back together would sort of defeat the point with IRQ domains to begin with. It would also make it more difficult to implement proper power management.

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Quoted 5 lines

That feeling may of course be completely wrong. Do you have a link to a data sheet describing how that controller actually works, or can you explain what the various arrays are needed for today, and how the various interrupt controllers you register fit together?

Sorry to say this, but I doubt that there are any public documents for our ARM based SoCs. Next time we meet face to face perhaps we can discuss about providing a board and documentation to you?

I'd be happy to try to describe how the INTC code works and how the sh7372 interrupt controllers are hooked together. Perhaps we can combine this with a chat for some more interactivity?

Quoted 5 lines · Magnus Damm

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

Quoted 8 lines

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Ah, so the vast majority of these are not for shmobile but for arch/sh and I support you have no plans to move those to DT along with shmobile, right?

Quoted 19 lines · Magnus Damm

Quoted 4 lines

Hi Magnus,

I'm trying to find my way through your patches, but I still have a little trouble figuring out how it all fits together.

Right, sorry for spitting out patches to the left and to the right. =)

So the full dependencies for "base DT support" are:

renesas.git a6e24019468009a21b674e392d74283a90f415dd (origin/master) [PATCH 00/06] mach-shmobile device tree preparation patches V2 [PATCH] ARM: mach-shmobile: sh7372 generic board support via DT V2

The patches above are rather ready IMO, and they've got Signed-off-by.

To get prototype level support of IRQs you will also need the following:
[PATCH] sh: INTC IRQ domain and DT support prototype
[PATCH] ARM: mach-shmobile: sh7372 DT IRQ prototype

Quoted 3 lines

The two patches above are experimental with Not-yet-signed-off-by.

There are more patches as well, but most depend on the prototype IRQ code above.

Do you have a public git tree that has all of them applied?

The separate patches are good for reviewing when one already knows the code, but in trying to get the big picture, I' prefer to look at the state after the patches.

Quoted 8 lines

Quoted 7 lines

My feeling is that the soc specific parts can be done better if you generalize the interrupt controller bindings so that you can describe the controller(s) as a single device, and allow all the information that is now in the intc-*.c files to be moved into device tree attributes to be parsed at boot time, or from the translate() function of the interrupt controller driver.

Quoted 8 lines

For sh7372 and most other of our SoCs I believe it will be difficult to use a single device for interrupts. This since there are a few different interrupt controllers working together, and they have different interrupt ranges associated with them. They fit quite well with IRQ domains and the ->dt_translate() callback and gluing them back together would sort of defeat the point with IRQ domains to begin with. It would also make it more difficult to implement proper power management.

Right. It wasn't clear to what degree they are actually separate bits of hardware. If the controllers are nested in some way, you should definitely describe them as individual device nodes.

Quoted 5 lines

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

Quoted 8 lines

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Ah, so the vast majority of these are not for shmobile but for arch/sh and I support you have no plans to move those to DT along with shmobile, right?

From my reading of the source code, your INTCA and INTCS controllers all follow the same basic model, but they all differ in the mapping between interrupt vectors to register locations.

I would probably not try to describe this mapping in the interrupt controller node, but instead use a more elaborate way to describe an interrupt than just the vector.

Taking the sh7372 USB0_USB0I0 interrupt as an arbitrary example, you could encode it as

       interrupts = <0x1ca0     /* vector */
                       0xa4 5   /* mask register offset and bit position */ 0x4c 1>; /* prio register offset and bit position */

An interrupt controller that only needs these (no sense/ack registers), would set #interrupt-cells=<5>;, while other interrupt controllers would use a larger number of interrupt cells to also encode the extra data.

You can also use the interrupt-map to turn these into more readable numbers for use by the devices.

I don't understand what your interrupt groups are, or whether you need even more attributes somewhere.

Quoted 6 lines

Quoted 5 lines

That feeling may of course be completely wrong. Do you have a link to a data sheet describing how that controller actually works, or can you explain what the various arrays are needed for today, and how the various interrupt controllers you register fit together?

Quoted 3 lines

Sorry to say this, but I doubt that there are any public documents for our ARM based SoCs. Next time we meet face to face perhaps we can discuss about providing a board and documentation to you?

Sounds good. I'll be at LinuxCon Japan in June, after the Hong Kong Linaro Connect.

Quoted 3 lines

I'd be happy to try to describe how the INTC code works and how the sh7372 interrupt controllers are hooked together. Perhaps we can combine this with a chat for some more interactivity?

Ok, let's try to meet up on IRC on #armlinux on freenode then.

Quoted 15 lines · Arnd Bergmann

Quoted 5 lines · Magnus Damm

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

Quoted 8 lines

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Quoted 4 lines

Ah, so the vast majority of these are not for shmobile but for arch/sh and I support you have no plans to move those to DT along with shmobile, right?

At the moment the main priority is really just the shared SoC blocks, so things like the interrupt controllers, serial driver, pfc/pinmux/gpios, dmaengine, etc. The interrupt controller subystem that we use for all of our cases is sufficiently complex that it's really not an ideal choice in terms of low-hanging fruit for DT support, in fact, it's probably the worst possible place to start.

I'm in the middle of irqdomain refactoring for it now, which will also entail quite a lot of rework with regards to how we deal with our virtual IRQs, radix tree utilization, and so forth. What sort of information we need to still keep around ultimately won't be known until we have a clearer picture of how the irqdomain tie-in will look for the existing cases. Only after that does it really make any sense to start on anything but the most trivial of DT bindings.

Beyond that, while the vast majority of the INTC cases are in arch/sh today, infrastructure-wise everything is sufficiently coupled together that it wouldn't make things any simpler to treat INTCA/INTCS specially.

Once we start seeing DT bindings in shared infrastructure (especially drivers/sh and so on) then I'll probably start gradually migrating arch/sh users over to it, too. For the majority of users we don't really have any sort of coherent boot ABI, which means that we're more than likely going to always have to link the blob in to the kernel directly, but that's another matter.

The GPIO/PFC pinmux/pinctrl bits are likewise quite complex and in flux while I work on pinctrl migration (some work done now, but more realistically after the irqdomain rework has happened), so it's not worth spending any time on DT bindings there for the moment either.

At the moment pretty much any other platform device is orders of magnitude more simplistic and workable though. I'm not sure why DT prototyping didn't start there, instead.

Quoted 14 lines

Quoted 5 lines

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

Quoted 8 lines

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Quoted 3 lines

Ah, so the vast majority of these are not for shmobile but for arch/sh and I support you have no plans to move those to DT along with shmobile, right?

Most use cases are still on SH, but for mach-shmobile we make use of either GIC of INTCA together with a cascaded INTCS. We also have IRQ pins that can be controlled from either INTCA or INTCS or just level/edge trigger configuration in front of GIC. In some cases we have an interrupt controller called PINT. Some PFC/GPIO controllers have built-in interrupt controllers, and some GPIOs have their interrupts tied into the INTC as IRQ pins. There is a big jambalaya of hardware IP with history from Mitsubishi, Hitachi and NEC, plus the usual not-so-exotic ones.

So with this in mind I believe we will need INTC support for mach-shmobile in the future as well. The INTC controllers seem to become more like leaf nodes these days though. =)

Quoted 3 lines

From my reading of the source code, your INTCA and INTCS controllers all follow the same basic model, but they all differ in the mapping between interrupt vectors to register locations.

All INTC tables are unique. So the INTCS in sh7372 is not the same as the INTCS in sh7367. Some parts of the tables may look similar, but low hanging fruit such as external IRQ pin handling has been broken out into reusable components already.

Quoted 3 lines

I would probably not try to describe this mapping in the interrupt controller node, but instead use a more elaborate way to describe an interrupt than just the vector.

That's certainly one way to split out the information, but I wonder if that's the right way to go. I worry that with such setup the correctness of the mappings between vectors and register fields is dependent on the system integrator doing proper copy-paste "development". One character wrong and you will have a silent error somewhere with the wrong interrupt being masked by accident for instance.

Quoted 10 lines

Taking the sh7372 USB0_USB0I0 interrupt as an arbitrary example, you could encode it as

       interrupts = <0x1ca0     /* vector */
                       0xa4 5   /* mask register offset and bit position */
                       0x4c 1>; /* prio register offset and bit position */

An interrupt controller that only needs these (no sense/ack registers), would set #interrupt-cells=<5>;, while other interrupt controllers would use a larger number of interrupt cells to also encode the extra data.

Thanks for showing this example. I think your proposal is interesting. I believe I get the big picture, but I wonder if it is that simple after all. In this specific example the USB0_USB0I0 interrupt source also needs a 0xe4 associated with the mask register. Some mask registers come in pairs with one clear and one set register. We also have the width of the register and width of the priority field as data. Of course the actual register mapping with such information could be kept together with the INTC configuration data for the interrupt controller, and we can for instance use register offset and bit field index to locate the actual bit field or bit. This becomes quite close to how INTC is working today though, with the exception that we use enums with the same names as the data sheet. I don't see how this can be done in Device Tree lingo.

Quoted 2 lines

I don't understand what your interrupt groups are, or whether you need even more attributes somewhere.

The groups are used to link together multiple interrupt sources (enums) that share a priority setting.

Anyway, just to be clear, at this point i mainly wanted to enable DT support on the mach-shmobile subarch and sh7372 in particular. So in my mind this excludes reworking the INTC and PFC tables. Such activities will take quite some time due to them being shared with SH. Of course we should work in that direction, but at this point I 'd like to focus on getting other people to work with DT support in the drivers. So because of that I spent time on getting interrupts going with the device tree instead of figuring out how to replace the INTC tables with something else. Step by step.

I assume that your current merge policy is that boards should be implemented in DT, but we can still merge new SoCs with current style of INTC and PFC tables as usual?

Quoted 15 lines · Arnd Bergmann

Quoted 5 lines · Magnus Damm

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

Quoted 8 lines

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Quoted 4 lines

Ah, so the vast majority of these are not for shmobile but for arch/sh and I support you have no plans to move those to DT along with shmobile, right?

At the moment the main priority is really just the shared SoC blocks, so things like the interrupt controllers, serial driver, pfc/pinmux/gpios, dmaengine, etc. The interrupt controller subystem that we use for all of our cases is sufficiently complex that it's really not an ideal choice in terms of low-hanging fruit for DT support, in fact, it's probably the worst possible place to start.

I'm in the middle of irqdomain refactoring for it now, which will also entail quite a lot of rework with regards to how we deal with our virtual IRQs, radix tree utilization, and so forth. What sort of information we need to still keep around ultimately won't be known until we have a clearer picture of how the irqdomain tie-in will look for the existing cases. Only after that does it really make any sense to start on anything but the most trivial of DT bindings.

Quoted 28 lines · Magnus Damm

Quoted 14 lines

Quoted 5 lines

However, the idea that you'd like to describe the controller with the device tree is interesting. At this point I'm unsure how to do that. Also, I do wonder if it makes sense to do so at all - most new parts use the GIC as the main interrupt controller anyway. I do agree with the plan of separating configuration from code, and in our INTC case

Quoted 8 lines

we've sort of already done so about 5 years ago. At that point I converted a few separate random implementations to the current table driven INTC code base. Since then we have soon close to 100 different users in arch/sh and arch/arm/mach-shmobile. Each is different. Feel free to grep for "register_intc_controller" to explore by yourself. It would be fun to try to use the device tree for this, but I wonder how we can describe anything half-complex without a preprocessor. Both the INTC code and the PFC used for GPIO and pinmux use C enums a lot.

Quoted 3 lines

Ah, so the vast majority of these are not for shmobile but for arch/sh and I support you have no plans to move those to DT along with shmobile, right?

Most use cases are still on SH, but for mach-shmobile we make use of either GIC of INTCA together with a cascaded INTCS. We also have IRQ pins that can be controlled from either INTCA or INTCS or just level/edge trigger configuration in front of GIC. In some cases we have an interrupt controller called PINT. Some PFC/GPIO controllers have built-in interrupt controllers, and some GPIOs have their interrupts tied into the INTC as IRQ pins. There is a big jambalaya of hardware IP with history from Mitsubishi, Hitachi and NEC, plus the usual not-so-exotic ones.

Quoted 3 lines

So with this in mind I believe we will need INTC support for mach-shmobile in the future as well. The INTC controllers seem to become more like leaf nodes these days though. =)

Ok, I see

Quoted 8 lines

Quoted 3 lines

From my reading of the source code, your INTCA and INTCS controllers all follow the same basic model, but they all differ in the mapping between interrupt vectors to register locations.

All INTC tables are unique. So the INTCS in sh7372 is not the same as the INTCS in sh7367. Some parts of the tables may look similar, but low hanging fruit such as external IRQ pin handling has been broken out into reusable components already.

What I meant was that the table structure is always the same, even if the contents are are different. It's fairly easy to replace data from the kernel sources with data in the device tree, but replacing code from the kernel with data is much harder.

Quoted 11 lines

Quoted 3 lines

I would probably not try to describe this mapping in the interrupt controller node, but instead use a more elaborate way to describe an interrupt than just the vector.

That's certainly one way to split out the information, but I wonder if that's the right way to go. I worry that with such setup the correctness of the mappings between vectors and register fields is dependent on the system integrator doing proper copy-paste "development". One character wrong and you will have a silent error somewhere with the wrong interrupt being masked by accident for instance.

All the bits describing the interrupt controller node specific to one soc should be able to go into the .dtsi file for that soc, so that a system integrator only needs to hook up the devices that are outside of the soc.

Quoted 11 lines

Quoted 10 lines

Taking the sh7372 USB0_USB0I0 interrupt as an arbitrary example, you could encode it as

       interrupts = <0x1ca0     /* vector */
                       0xa4 5   /* mask register offset and bit position */ 0x4c 1>; /* prio register offset and bit position */

An interrupt controller that only needs these (no sense/ack registers), would set #interrupt-cells=<5>;, while other interrupt controllers would use a larger number of interrupt cells to also encode the extra data.

Quoted 13 lines

Thanks for showing this example. I think your proposal is interesting. I believe I get the big picture, but I wonder if it is that simple after all. In this specific example the USB0_USB0I0 interrupt source also needs a 0xe4 associated with the mask register. Some mask registers come in pairs with one clear and one set register. We also have the width of the register and width of the priority field as data. Of course the actual register mapping with such information could be kept together with the INTC configuration data for the interrupt controller, and we can for instance use register offset and bit field index to locate the actual bit field or bit. This becomes quite close to how INTC is working today though, with the exception that we use enums with the same names as the data sheet. I don't see how this can be done in Device Tree lingo.

There are two places where you could put that information. The one I described above is the interrupts property, but if you have to add many more fields, this gets impractical.

The other place would be sub-nodes of the interrupt controller node that don't get translated into individual platform devices.

You can have one node per interrupt line, like this:

       intc@6940000 {
               #address-cells = <2>;
               #size-cells = <0>;
               reg = <0x6940000 0x1000>;
               ranges = <1 0  0x6940000 0x54   /* prio */ 2 0  0x6940080 0x3c   /* mask */ 3 0  0x69400c0 0x3c>; /* unmask /

               /* common properties for priority nodes, could get overridden if necessary */ prio-reg-width = <16>; prio-width = <4>;

               iprta3 {
                       #address-cells = <2>;
                       #size-cells = <0>;
                       ranges;

                       reg = <1 0x4c> /* priority reg 0x4c */

                       usb0_usb0i0 {
                               reg = <0 0x1ca0  /* vector 0x1ca0 */ 1 0x34   /* mask   0x80 + 0x34 */ 2 0x34>; /* unmask 0xc0 + 0x34 */ reg-mask = <0x04>; /* bitmask for registers */ };
               };
       };

Quoted 16 lines

Quoted 2 lines

I don't understand what your interrupt groups are, or whether you need even more attributes somewhere.

The groups are used to link together multiple interrupt sources (enums) that share a priority setting.

Anyway, just to be clear, at this point i mainly wanted to enable DT support on the mach-shmobile subarch and sh7372 in particular. So in my mind this excludes reworking the INTC and PFC tables. Such activities will take quite some time due to them being shared with SH. Of course we should work in that direction, but at this point I 'd like to focus on getting other people to work with DT support in the drivers. So because of that I spent time on getting interrupts going with the device tree instead of figuring out how to replace the INTC tables with something else. Step by step.

Quoted 3 lines

I assume that your current merge policy is that boards should be implemented in DT, but we can still merge new SoCs with current style of INTC and PFC tables as usual?

Yes, that sounds ok. The part that I would not want to see though is having a preliminary binding for INTC that you later have to replace by another binding, breaking all device tree files that were produced at first.

It's ok if you add more data to the intc nodes at a later point while incrementally moving stuff into dts files, but I think you should decide on how to represent an interrupt line in the devices using intc.