Page MenuHomeFreeBSD

Bring MIPS INTRNG support up to date
ClosedPublic

Authored by sgalabov on Apr 5 2016, 1:01 PM.

Details

Summary

D5730 changed the INTRNG PIC interface, so as a result MIPS INTRNG became unusable.
This revision aims to bring MIPS INTRNG support back again and at the level defined by D5370.

Test Plan

Tested on MT7688 LinkIt.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sgalabov retitled this revision from to Bring MIPS INTRNG support up to date.Apr 5 2016, 1:01 PM
sgalabov updated this object.
sgalabov edited the test plan for this revision. (Show Details)
sgalabov added reviewers: adrian, kan, skra.
sgalabov set the repository for this revision to rS FreeBSD src repository.
sgalabov added a project: MIPS.
sgalabov updated this revision to Diff 14885.
kan added inline comments.Apr 5 2016, 1:41 PM
sys/mips/mips/mips_pic.c
176 ↗(On Diff #14885)

This needs to be addressed before commit?

209 ↗(On Diff #14885)

0, sc->nirqs - 1 ?

sgalabov updated this revision to Diff 14895.Apr 5 2016, 1:55 PM

Updated diffs to reflect kan@'s comments.

sgalabov marked 2 inline comments as done.Apr 5 2016, 1:55 PM
kan added inline comments.Apr 5 2016, 4:16 PM
sys/mips/mips/mips_pic.c
83 ↗(On Diff #14895)

Come to think of it, what is the purpose of rman here? It serves to useful purpose but to provide a fake struct resource for cpu_establish_hard|soft_intr. I would just kill it off for now - it does not look like INTRNG plugs into resource allocation properly anyway.

adrian edited edge metadata.Apr 5 2016, 4:20 PM
adrian accepted this revision.

thanks!

This revision is now accepted and ready to land.Apr 5 2016, 4:20 PM
skra added inline comments.Apr 5 2016, 5:17 PM
sys/mips/mips/mips_pic.c
83 ↗(On Diff #14895)

Another solution is to change interrupt number argument to struct resource pointer in cpu_establish_hardintr() and cpu_establish_softintr(). And whoever wants to use these functions should get a struct resource by call to bus_allocate_resource().

BTW, INTRNG doesn't plug into resource allocation properly only for interrupt numbers get from ofw_bus_map_intr(). And it should be fixed soon.

sgalabov added inline comments.Apr 5 2016, 6:13 PM
sys/mips/mips/mips_pic.c
83 ↗(On Diff #14895)

That's true, but I couldn't think of a cleaner way to satisfy intr_setup_irq()'s requirement for a struct resource * parameter. And although I could go and change all users of cpu_establish_*_intr, this seems even more troublesome at the moment.
So, if you guys don't mind, I'd like to leave the rman as it is, even though it's a bit of an overkill.

adrian added a comment.Apr 5 2016, 6:19 PM

I'm happy with it being as-is for now.

skra added inline comments.Apr 5 2016, 7:12 PM
sys/mips/mips/mips_pic.c
83 ↗(On Diff #14895)

I'm okay with it, even if it's quite weird. It was just a note from me. However, INTRNG is meant to be used by bus drivers in their bus methods where struct resource is always present. So if it's used another way, curious things come.

skra edited edge metadata.Apr 5 2016, 7:21 PM

Thanks that you are doing this. I thought that I would do this later, as there was no immediate need to do it in D5730. Thus, thanks again and sorry for trouble.

In D5838#124795, @skra wrote:

Thanks that you are doing this. I thought that I would do this later, as there was no immediate need to do it in D5730. Thus, thanks again and sorry for trouble.

No problem. I am relying on INTRNG for the Ralink/Mediatek support that I'm starting to push into -head, so I needed to do it anyway :-) I hope I've done it right.

kan edited edge metadata.Apr 7 2016, 10:31 AM

OK, this can go in as is for now.

kan edited edge metadata.Apr 7 2016, 10:31 AM
kan accepted this revision.
This revision was automatically updated to reflect the committed changes.