Make MPIC compatible with ARM_INTRNG

[bsz_semihalf.com]

Summary

After ARM_INTRNG introduction, MPIC code needed several modifications:

• IRQ resource and its handler added
• several DEVMETHODs of INTRNG interface implemented
• defines enhanced to ensure code compiles as well for AXP as for A38X
• added dummy MSI_IRQ, ERR_IRQ defines for Armada38x

MPIC driver was added to files.armada38x, ARM_INTRNG option enabled in
kernconf file and regs of MPIC corrected in dts file.

Instead of modifying Armada38X DTS, offsets to CPU registers defined in
driver were changed. That required restoring 'reg' property of mpic node
in ArmadaXP to state compliant with Linux DTS.

Additionally, required ARM_INTRNG definitions were added to mv_common.c.

Obtained from: Semihalf
Sponsored by: Stormshield
Submitted by: Bartosz Szczepanek <[e-mail]>

[andrew]

sys/arm/mv/mpic.c

166–167 ↗

(On Diff #12591)

Why not ofw_bus_search_compatible?

181 ↗

(On Diff #12591)

Why not OF_xref_from_device? This should also not be set here, you should move the call to where it's used.

237 ↗

(On Diff #12591)

What about when the next SoC is added? It would be better to check for a list of SoCs that need to call this function rather than the ones that don't.

398 ↗

(On Diff #12591)

Why _LATE?

[bsz_semihalf.com]

sys/arm/mv/mpic.c

398 ↗

(On Diff #12591)

I thought it would be required as MPIC is slave interrupt controller (thus requires GIC to be attached before), but I tested it and it works without _LATE as well. I'll remove that.

[bsz_semihalf.com]

Corrected to follow @andrew 's remarks.

[bsz_semihalf.com]

@andrew, is it OK? Please accept if so.

[andrew]

sys/arm/mv/mpic.c

36 ↗

(On Diff #12716)

What is this header for?

118 ↗

(On Diff #12716)

Why is this shareable?

[skra]

sys/arm/include/intr.h

56 ↗

(On Diff #12716)

So, there is no other way but put SOC specific things into common header?
These defines are used only in one file.

174 ↗

(On Diff #12716)

Same as above.

sys/arm/mv/mpic.c

259 ↗

(On Diff #12716)

PCPU_GET(cpuid)

271 ↗

(On Diff #12716)

What about to mask the interrupt? Stray interrupt can block all system.

[andrew]

sys/arm/mv/mpic.c

400 ↗

(On Diff #12716)

Normally we use _MIDDLE to allow other devices to attach earlier or latter if needed.

[• ian]

sys/arm/mv/mpic.c

239 ↗

(On Diff #12716)

This is the sort of thing the ocd_data field is good for in the compat_data struct, if the different behavior is signaled by a different compat string (and if it behaves differently and has the same compat, then somebody is screwing up the fdt data on you).

Instead of just true/false you can put a set of bit-mapped options such as IS_MPIC|NEEDS_MSI_UNMASK, then when probing you can check the IS_MPIC bit and when attaching you save all of them in some variable like sc_options or whatever for later reference.

400 ↗

(On Diff #12716)

With INTRNG it probably doesn't matter which order the two interrupt controllers attach. Part of the design of INTRNG is to allow interrupt assignments for PICs that aren't attached (and maybe never even will attach, such as with an i2c gpio/interrupt expander).

[bsz_semihalf.com]

sys/arm/include/intr.h

174 ↗

(On Diff #12716)

I am not sure what is the right place for these defines. Should I move these to std file? @ian?

sys/arm/mv/mpic.c

36 ↗

(On Diff #12716)

Without that there is no "FDT" defined, what excludes isrc_ncells and isrc_cells from intr_irqsrc structure (sys/arm/include/intr.h). That causes compilation errors in mpic_map_fdt.
I think that the latter should be in #ifdef FDT block as well. The header must be included then.

239 ↗

(On Diff #12716)

I am not sure if this is the case. We have "marvell,mpic" compat string and it may be the same chip everywhere, but I suppose it's wired differently on A38X - only 0-31 interrupts from slave IC are usable. So this is rather platform-dependent.

In Linux additional property 'msi-controller' exists. This is enabled on both A38X and AXP, on the contrary to A38X documentation, which doesn't say a word about MSI interrupt on MPIC.

However, removing the ifdef from here doesn't do a harm, maybe that's the solution.

[bsz_semihalf.com]

Few corrections.

[adrian]

hiya,

I just refactored out stuff from sys/arm/include/intr.h into sys/intr.h; please update the diff and let me know if anything else needs changing. :)

THanks!

-a

[bsz_semihalf.com]

Moved IRQ definitions to platform header.

[bsz_semihalf.com]

Thank you for your remarks - diff was updated accordingly. If there are no objections, we will commit that.