soundwire: SDCA: Generic interrupt support#5365
soundwire: SDCA: Generic interrupt support#5365mstrozek wants to merge 2 commits intothesofproject:topic/sof-devfrom
Conversation
|
Thanks @mstrozek It looks quite decent to me. I will pause my SDCA interrupt work and check if Realtek codec drivers can also use the helpers. @shumingfan FYI |
|
I think there might still be some issues with this patch, having some issues getting interrupts with it. Will update more once I have investigated a bit more. |
mstrozek
force-pushed
the
sdca-interrupts
branch
3 times, most recently
from
bc81b51 to
94f29d7
Compare
|
Also seems to be some issues when you have multiple IRQs enabled just hunting that down will update but feeling like it is likely going to be tomorrow. |
|
Ok I think the multiple IRQ thing was just caching on the regmap, so shouldn't affect this patch. Looks like I have basic IRQ functionality going on Phife (with my outstanding comments here). Will keep poking it all tomorrow. |
mstrozek
force-pushed
the
sdca-interrupts
branch
from
94f29d7 to
3fe45d1
Compare
include/sound/sdca_interrupts.h
Outdated
| struct regmap_irq_chip_data *irq_data; | ||
| struct irq_domain *irq_dom; | ||
|
|
||
| struct mutex irqs_lock; /* protects SDCA interrupts */ |
There was a problem hiding this comment.
is this used and needed? I remember I had a good reason to add this lock but I can't remember which one....
There was a problem hiding this comment.
It is going to be needed for callbacks. Removed it now since nothing uses it directly, is it ok?
sound/soc/sdca/sdca_interrupts.c
Outdated
| IRQ_SDCA(27, 4), | ||
| IRQ_SDCA(28, 4), | ||
| IRQ_SDCA(29, 4), | ||
| IRQ_SDCA(30, 4), |
There was a problem hiding this comment.
The register number seems overkill, it can be found directly from the interrupt number
There was a problem hiding this comment.
I thought it was easier to read with the reg number separate, but changed now to use only the irq number
sound/soc/sdca/sdca_interrupts.c
Outdated
| case IRQF_TRIGGER_RISING: | ||
| case IRQF_TRIGGER_FALLING: | ||
| break; | ||
| case IRQ_TYPE_NONE: |
There was a problem hiding this comment.
I can't figure out how those flags would be used. Can we really support all of this?
There was a problem hiding this comment.
Probably not, you are right. Forced it now to a low level trigger, I guess this is closest in behaviour to soundwire's alert bit?
include/sound/sdca.h
Outdated
| #include <linux/types.h> | ||
| #include <linux/kconfig.h> | ||
| #include <linux/regmap.h> | ||
| #include <linux/interrupt.h> |
There was a problem hiding this comment.
Commit message needs more explanations and removed change-id
There was a problem hiding this comment.
Commit msg updated, is the wording ok?
mstrozek
force-pushed
the
sdca-interrupts
branch
2 times, most recently
from
80304a2 to
38d9e9b
Compare
Zero is a valid SDCA IRQ interrupt position so add a special value to indicate that the IRQ is not used. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Add a library supporting usage of SDCA interrupts, using regmap irq framework. The library adds functions for parsing ACPI for interrupt-related information, configuring irq chip and requesting individual irqs. Calling code (SDCA function code) is expected to also substitute the library's base irq handler for its own, appropriate callback. Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
mstrozek
force-pushed
the
sdca-interrupts
branch
from
38d9e9b to
3f81023
Compare
|
One last question I have, are we sure this should be a separate module, and should it be user selectable or selected by the the drivers using it? I am leaning towards keeping it as a separate module, but making it not user-selectable? |
|
@mstrozek @charleskeepax Do you plan to submit the series upstream? |
|
@bardliao Yes, most likely next week after the merge window closes |
|
Close it as it is already upstream |
5 participants
Generic interrupts using the kernel's regmap irqs