The Linux Kernel Mailing List
 help / color / mirror / Atom feed
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

  reply	other threads:[~2026-07-20 15:03 UTC|newest]

Thread overview: 12+ 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-21 10:30       ` 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