From: Vinod Koul <vkoul@kernel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
tiwai@suse.de, broonie@kernel.org, gregkh@linuxfoundation.org,
jank@cadence.com, srinivas.kandagatla@linaro.org,
slawomir.blauciak@intel.com,
Bard liao <yung-chuan.liao@linux.intel.com>,
Rander Wang <rander.wang@linux.intel.com>,
Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
Sanyog Kale <sanyog.r.kale@intel.com>
Subject: Re: [PATCH 3/3] soundwire: ignore uniqueID when irrelevant
Date: Thu, 24 Oct 2019 17:09:11 +0530 [thread overview]
Message-ID: <20191024113911.GD2620@vkoul-mobl> (raw)
In-Reply-To: <20191022234808.17432-4-pierre-louis.bossart@linux.intel.com>
On 22-10-19, 18:48, Pierre-Louis Bossart wrote:
> The uniqueID is useful when there are two or more devices of the same
> type (identical manufacturer ID, part ID) on the same link.
Right!
> When there is a single device of a given type on a link, its uniqueID
> is irrelevant. It's not uncommon on actual platforms to see variations
> of the uniqueID, or differences between devID registers and ACPI _ADR
> fields.
Ideally this should be fixed in firmware, I do not like the fact the we are
poking in core for firmware issues!
> This patch suggests a filter on startup to identify 'single' devices
> and tag them accordingly.
So you try to see if the board has a single device and mark them so that
you can skip the unique id, did I get that right?
What about the boards which have multiple devices? How doing solve
these?
> The uniqueID is then not used for the probe,
> and the device name omits the uniqueID as well.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> drivers/soundwire/bus.c | 7 +++---
> drivers/soundwire/slave.c | 52 ++++++++++++++++++++++++++++++++++++---
> 2 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index fc53dbe57f85..be5d437058ed 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -422,10 +422,11 @@ static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)
>
> static int sdw_compare_devid(struct sdw_slave *slave, struct sdw_slave_id id)
> {
> - if (slave->id.unique_id != id.unique_id ||
> - slave->id.mfg_id != id.mfg_id ||
> + if (slave->id.mfg_id != id.mfg_id ||
> slave->id.part_id != id.part_id ||
> - slave->id.class_id != id.class_id)
> + slave->id.class_id != id.class_id ||
> + (slave->id.unique_id != SDW_IGNORED_UNIQUE_ID &&
> + slave->id.unique_id != id.unique_id))
> return -ENODEV;
>
> return 0;
> diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
> index 5dbc76772d21..19919975bb6d 100644
> --- a/drivers/soundwire/slave.c
> +++ b/drivers/soundwire/slave.c
> @@ -29,10 +29,17 @@ static int sdw_slave_add(struct sdw_bus *bus,
> slave->dev.parent = bus->dev;
> slave->dev.fwnode = fwnode;
>
> - /* name shall be sdw:link:mfg:part:class:unique */
> - dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x:%x",
> - bus->link_id, id->mfg_id, id->part_id,
> - id->class_id, id->unique_id);
> + if (id->unique_id == SDW_IGNORED_UNIQUE_ID) {
> + /* name shall be sdw:link:mfg:part:class */
> + dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x",
> + bus->link_id, id->mfg_id, id->part_id,
> + id->class_id);
> + } else {
> + /* name shall be sdw:link:mfg:part:class:unique */
> + dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x:%x",
> + bus->link_id, id->mfg_id, id->part_id,
> + id->class_id, id->unique_id);
> + }
>
> slave->dev.release = sdw_slave_release;
> slave->dev.bus = &sdw_bus_type;
> @@ -103,6 +110,7 @@ static bool find_slave(struct sdw_bus *bus,
> int sdw_acpi_find_slaves(struct sdw_bus *bus)
> {
> struct acpi_device *adev, *parent;
> + struct acpi_device *adev2, *parent2;
>
> parent = ACPI_COMPANION(bus->dev);
> if (!parent) {
> @@ -112,10 +120,46 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)
>
> list_for_each_entry(adev, &parent->children, node) {
> struct sdw_slave_id id;
> + struct sdw_slave_id id2;
> + bool ignore_unique_id = true;
>
> if (!find_slave(bus, adev, &id))
> continue;
>
> + /* brute-force O(N^2) search for duplicates */
> + parent2 = parent;
> + list_for_each_entry(adev2, &parent2->children, node) {
> +
> + if (adev == adev2)
> + continue;
> +
> + if (!find_slave(bus, adev2, &id2))
> + continue;
> +
> + if (id.sdw_version != id2.sdw_version ||
> + id.mfg_id != id2.mfg_id ||
> + id.part_id != id2.part_id ||
> + id.class_id != id2.class_id)
> + continue;
> +
> + if (id.unique_id != id2.unique_id) {
> + dev_dbg(bus->dev,
> + "Valid unique IDs %x %x for Slave mfg %x part %d\n",
> + id.unique_id, id2.unique_id,
> + id.mfg_id, id.part_id);
> + ignore_unique_id = false;
> + } else {
> + dev_err(bus->dev,
> + "Invalid unique IDs %x %x for Slave mfg %x part %d\n",
> + id.unique_id, id2.unique_id,
> + id.mfg_id, id.part_id);
> + return -ENODEV;
> + }
> + }
> +
> + if (ignore_unique_id)
> + id.unique_id = SDW_IGNORED_UNIQUE_ID;
> +
> /*
> * don't error check for sdw_slave_add as we want to continue
> * adding Slaves
> --
> 2.20.1
--
~Vinod
next prev parent reply other threads:[~2019-10-24 11:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-22 23:48 [PATCH 0/3] soundwire: use UniqueID only when relevant Pierre-Louis Bossart
2019-10-22 23:48 ` [PATCH 1/3] soundwire: remove bitfield for unique_id, use u8 Pierre-Louis Bossart
2019-10-24 11:29 ` Vinod Koul
2019-10-24 12:42 ` [alsa-devel] " Pierre-Louis Bossart
2019-10-22 23:48 ` [PATCH 2/3] soundwire: slave: add helper to extract slave ID Pierre-Louis Bossart
2019-10-22 23:48 ` [PATCH 3/3] soundwire: ignore uniqueID when irrelevant Pierre-Louis Bossart
2019-10-24 11:39 ` Vinod Koul [this message]
2019-10-24 12:59 ` [alsa-devel] " Pierre-Louis Bossart
2019-11-06 19:30 ` [alsa-devel] [PATCH 0/3] soundwire: use UniqueID only when relevant Pierre-Louis Bossart
2019-11-09 11:18 ` Vinod Koul
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=20191024113911.GD2620@vkoul-mobl \
--to=vkoul@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jank@cadence.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=rander.wang@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=sanyog.r.kale@intel.com \
--cc=slawomir.blauciak@intel.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=tiwai@suse.de \
--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;
as well as URLs for NNTP newsgroup(s).