![](https://secure.gravatar.com/avatar/59bffa8d29505a5951521a2fd09a4fd9.jpg?s=120&d=mm&r=g)
On 6/8/23 02:06, Vinod Koul wrote:
On 31-05-23, 11:37, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This patch adds a new Device Number allocation strategy, with the IDA used only for devices that are wake-capable.
"regular" devices such as amplifiers will use Device Numbers [1..min_ida-1].
"wake-capable" devices such as jack or microphone codecs will use Device Numbers [min_ida..11].
This hybrid strategy extends the number of supported devices in a system by only constraining the allocation if required, e.g. in the case of Intel LunarLake platforms the wake-capable devices are required to have a unique address to use the HDaudio SDI and HDAudio WAKEEN/WAKESTS registers.
This seems to be a consequence of Intel hardware decisions, so I guess best suited place for this is Intel controller, do we really want to have this in core logic?
It's a valid objection.
The reason why I added the alternate strategies in the core logic is that the IDA and hybrid approach are just software-based with no specific hardware dependencies. If QCOM or AMD wanted to use the strategies contributed and tested by Intel, it'd be a two-line change on their side.
That said, it's likely that at some point *someone* will want to constrain the device number allocation further, be it with ACPI/DT properties or reading hardware registers. The device number is a de-facto priority given the way we scan the PING frames, so some systems may want to give a higher priority to a specific peripherals.
This would push us to add a master ops callback to control the device number allocation. It's a bit invasive but that would give the ultimate flexibility. Reuse between vendors could be possible if 'generic' callbacks were part of a library to pick from.
I don't really have any objections if this vendor-specific callback was preferred, it may be a bit early to add this but long-term it's probably what makes more sense.
I'll go with the flow on suggested recommendations.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
drivers/soundwire/bus.c | 26 +++++++++++++++++++++----- include/linux/soundwire/sdw.h | 4 ++++ 2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index e8c1c55a2a73..6f465cce8369 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -159,7 +159,9 @@ static int sdw_delete_slave(struct device *dev, void *data)
if (slave->dev_num) { /* clear dev_num if assigned */ clear_bit(slave->dev_num, bus->assigned);
if (bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA)
if (bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA ||
(bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA_WAKE_ONLY &&
} list_del_init(&slave->node);slave->prop.wake_capable)) ida_free(&sdw_peripheral_ida, slave->dev_num);
@@ -699,17 +701,31 @@ EXPORT_SYMBOL(sdw_compare_devid); /* called with bus_lock held */ static int sdw_get_device_num(struct sdw_slave *slave) {
- struct sdw_bus *bus = slave->bus; int bit;
- if (slave->bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA) {
- if (bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA ||
(bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA_WAKE_ONLY &&
bit = ida_alloc_range(&sdw_peripheral_ida,slave->prop.wake_capable)) {
slave->bus->dev_num_ida_min, SDW_MAX_DEVICES,
if (bit < 0) goto err; } else {bus->dev_num_ida_min, SDW_MAX_DEVICES, GFP_KERNEL);
bit = find_first_zero_bit(slave->bus->assigned, SDW_MAX_DEVICES);
if (bit == SDW_MAX_DEVICES) {
int max_devices = SDW_MAX_DEVICES;
if (bus->dev_num_alloc == SDW_DEV_NUM_ALLOC_IDA_WAKE_ONLY &&
!slave->prop.wake_capable) {
max_devices = bus->dev_num_ida_min - 1;
/* range check */
if (max_devices < 1 || max_devices > SDW_MAX_DEVICES)
return -EINVAL;
}
bit = find_first_zero_bit(bus->assigned, max_devices);
}if (bit == max_devices) { bit = -ENODEV; goto err;
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 4656d6d0f3bb..8a7541ac735e 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -869,10 +869,14 @@ struct sdw_master_ops {
- @SDW_DEV_NUM_ALLOC_DEFAULT: unconstrained first-come-first-serve allocation,
- using range [1, 11]
- @SDW_DEV_NUM_ALLOC_IDA: IDA-based allocation, using range [ida_min, 11]
- @SDW_DEV_NUM_ALLOC_IDA_WAKE_ONLY: Hybrid allocation where wake-capable devices rely on
- IDA-based allocation and range [ida_min, 11], while regular devices rely on default
*/
- allocation in range [1, ida_min - 1]
enum sdw_dev_num_alloc { SDW_DEV_NUM_ALLOC_DEFAULT = 0, SDW_DEV_NUM_ALLOC_IDA,
- SDW_DEV_NUM_ALLOC_IDA_WAKE_ONLY,
};
/**
2.25.1