From: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
To: Charles Keepax <ckeepax@opensource.cirrus.com>, vkoul@kernel.org
Cc: yung-chuan.liao@linux.intel.com, sanyog.r.kale@intel.com,
linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
patches@opensource.cirrus.com
Subject: Re: [PATCH 2/2] soundwire: bus: Add internal slave ID and use for IRQs
Date: Tue, 22 Apr 2025 13:50:13 +0200 [thread overview]
Message-ID: <b286bbf3-0da9-4e84-8d21-7720970833c3@linux.dev> (raw)
In-Reply-To: <20250422104245.958678-3-ckeepax@opensource.cirrus.com>
On 4/22/25 12:42, Charles Keepax wrote:
> Currently the SoundWire IRQ code uses the dev_num to create an IRQ
> mapping for each slave. However, there is an issue there, the dev_num
> is only allocated when the slave enumerates on the bus and enumeration
> may happen before or after probe of the slave driver. In the case
> enumeration happens after probe of the slave driver then the IRQ
> mapping will use dev_num before it is set. This could cause multiple
> slaves to use zero as their IRQ mapping.
>
> It is very desirable to have the IRQ mapped before the slave probe
> is called, so drivers can do resource allocation in probe as normal. To
> solve these issues add an internal ID created for each slave when it is
> probed and use that for mapping the IRQ. In the case that
> get_device_num() is not implemented this ID can also be reused for the
> dev_num.
Humm, I am missing something. Does this work in the case of spurious 'ghost' ACPI devices, which is quite common for Intel DSDT?
In this case, each 'ghost' device would be assigned a dev_num during the driver probe, but that dev_num would never be used for enumeration, would it?
Let's assume for the sake of the argument there are 16 ghost devices and only one real device that gets enumerated. How can we guarantee that the real device is enumerated with a dev_num <= 11 (the max number of devices supported on the bus)?
I see the patch add a limit during probe, so now that means the number of ACPI devices MUST be lower than 11. That doesn't sound right to me and could cause some configurations to fail. It's perfectly ok to have ghost devices and no limits on how many our BIOS colleagues decide to list.
Using a dedicated IDA for IRQ mapping looks like a good idea to me, but I don't think you can really use the same IDA for dev_num
next prev parent reply other threads:[~2025-04-22 11:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-22 10:42 [PATCH 0/2] Fix minor issue in SoundWire slave IRQ mapping Charles Keepax
2025-04-22 10:42 ` [PATCH 1/2] soundwire: bus: Simplify sdw_assign_device_num() Charles Keepax
2025-04-22 10:42 ` [PATCH 2/2] soundwire: bus: Add internal slave ID and use for IRQs Charles Keepax
2025-04-22 11:50 ` Pierre-Louis Bossart [this message]
2025-04-22 12:11 ` Charles Keepax
2025-04-25 16:35 ` Pierre-Louis Bossart
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=b286bbf3-0da9-4e84-8d21-7720970833c3@linux.dev \
--to=pierre-louis.bossart@linux.dev \
--cc=ckeepax@opensource.cirrus.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=sanyog.r.kale@intel.com \
--cc=vkoul@kernel.org \
--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