From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Cc: broonie@kernel.org, lgirdwood@gmail.com,
yung-chuan.liao@linux.intel.com, linux-sound@vger.kernel.org,
linux-kernel@vger.kernel.org, patches@opensource.cirrus.com,
"shumingf@realtek.com" <shumingf@realtek.com>
Subject: Re: [PATCH v3 2/4] ASoC: SDCA: Populate IRQ data earlier
Date: Mon, 20 Jul 2026 16:02:45 +0100 [thread overview]
Message-ID: <al44lZ+uzmPJW3Ul@opensource.cirrus.com> (raw)
In-Reply-To: <305305e9-6f41-46bf-96fa-0495ef27062a@linux.dev>
On Mon, Jul 20, 2026 at 04:44:08PM +0200, Pierre-Louis Bossart wrote:
> On 7/20/26 16:37, Charles Keepax wrote:
> > On Fri, Jul 17, 2026 at 10:42:46PM +0200, Pierre-Louis Bossart wrote:
> >> On 7/16/26 16:26, Charles Keepax wrote:
> >>> Currently, the IRQ data (attached Entity/Control/etc) is populated
> >>> as the IRQ is requested. However, this can cause issues as
> >>> occasionally the setup process wants to access specifics of
> >>> an IRQ before the IRQ is actually enabled. To facilitate this
> >>> cache all the IRQ data during sdca_irq_populate_early() and make
> >>> sdca_irq_populate() simply request the outstanding IRQs. This
> >>> also has the advantage that sdca_irq_populate() can now just
> >>> iterate through the IRQ array which is much smaller/faster than
> >>> going through every Entity in the Function for Controls.
> >>>
> >>> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> >>> ---
> >>
> >> I may be splitting hair but shouldn't this patch be moved
> >> earlier?patch1/3-4 seem to be completely tied with the introduction and
> >> use of the fixup_controls() callback, I am not sure I see the link with
> >> the IRQ stuff. Or conversely move this last if this makes more sense.
> >
> > It can be moved to the start of the series, probably not worth a
> > spin on its own but if we end up with changes from Mark's
> > comments I will do so.
> >
> > This is slightly related though as sdca_jack_set_jack is called
> > before all the controls are registered and we populate the jack
> > masks in there, so moving the IRQs to later means we need that
> > information ready at that time which this accomplishes. Although
> > it does also just generally make things a bit neater.
> >
> >> Also while I am at it, I wouldn't hurt to have an explanation of the
> >> races conditions earlier, it's only in patch3 that the race condition is
> >> described in details.
> >
> > Yeah I should have probably included that in the cover letter
> > more. Likewise, I will update that if we end up with another
> > spin. If either you or Mark feel strongly enough on these points
> > to warrant another spin just for them just let me know.
>
> I do have a new question triggered by the last patch from Shuming: what
> happens to the existing users of sdca_irq_populate(), do they also need
> to use the new sdca_irq_populate_early()? See my comment in the other
> patch "ASoC: rt766: add RT766/RT767 SDCA driver"
>
> If there is no impact no need for a new spin IMHO.
Well I updated the existing user :-)
But seriously, thanks for pointing that out I missed that
patch on the list. I will go do some review.
Thanks,
Charles
next prev parent reply other threads:[~2026-07-20 15:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-16 14:26 [PATCH v3 0/4] Fix races on creation of SDCA jack detection Charles Keepax
2026-07-16 14:26 ` [PATCH v3 1/4] ASoC: Add a component fixup_controls callback Charles Keepax
2026-07-16 14:26 ` [PATCH v3 2/4] ASoC: SDCA: Populate IRQ data earlier Charles Keepax
2026-07-17 20:42 ` Pierre-Louis Bossart
2026-07-20 14:37 ` Charles Keepax
2026-07-20 14:44 ` Pierre-Louis Bossart
2026-07-20 15:02 ` Charles Keepax [this message]
2026-07-19 13:24 ` Mark Brown
2026-07-20 14:31 ` Charles Keepax
2026-07-16 14:26 ` [PATCH v3 3/4] ASoC: SDCA: Add sdca_irq_cleanup_late() Charles Keepax
2026-07-16 14:26 ` [PATCH v3 4/4] ASoC: SDCA: Switch to fixup_controls callback for IRQ registration Charles Keepax
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=al44lZ+uzmPJW3Ul@opensource.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=pierre-louis.bossart@linux.dev \
--cc=shumingf@realtek.com \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox