On 03/03/2021 16:35, Pierre-Louis Bossart wrote:
On 3/3/21 3:38 AM, Srinivas Kandagatla wrote:
On 02/03/2021 14:34, Pierre-Louis Bossart wrote:
+ if (!val1 && !val2)
+ break;
+
+ addr = buf2[1] | (buf2[0] << 8) | (buf1[3] << 16) |
+ ((u64)buf1[2] << 24) | ((u64)buf1[1] << 32) |
+ ((u64)buf1[0] << 40);
+
+ sdw_extract_slave_id(bus, addr, &id);
+ /* Now compare with entries */
+ list_for_each_entry_safe(slave, _s, &bus->slaves, node) {
+ if (sdw_compare_devid(slave, id) == 0) {
+ u32 status = qcom_swrm_get_n_device_status(ctrl, i);
+ if (status == SDW_SLAVE_ATTACHED) {
+ slave->dev_num = i;
+ mutex_lock(&bus->bus_lock);
+ set_bit(i, bus->assigned);
+ mutex_unlock(&bus->bus_lock);
+
+ }
And that part is strange as well. The bus->assigned bit should be
set even if the Slave is not in the list provided by platform
firmware. It's really tracking the state of the hardware, and it
should not be influenced by what software knows to manage.
Am not 100% sure If I understand the concern here, but In normal
(non auto enum) cases this bit is set by the bus code while its
doing enumeration to assign a dev number from the assigned bitmap!
However in this case where auto enumeration happens it makes sense
to set this here with matching dev number!
AFAIU from code, each bit in this bitmap corresponds to slave dev
number!
Yes, but the point was "why do you compare with information coming
from platform firmware"? if the hardware reports the presence of
devices on
This is the logic that hardware IP document suggests to use to get get
the correct the device number associated with the slave!
the link, why not use the information as is?
You recently added code that helps us deal with devices that are not
listed in DT or ACPI tables, so why would we filter in this specific
loop?
I don't think my point was understood, so let me try to explain it
differently.
it's my understanding that the hardware reads the DevID registers and
writes a Device Number - so that the entire enumeration sequence started
by reading DevID0 and finished by a successful write to SCP_DevNum is
handled in hardware.
The question is: what happens if that device is NOT described in the
Device Tree data? The loop over bus->slaves will not find this device by
comparing with known devID values, so the set_bit(i, bus->assigned) will
not happen.
yes, that is true, There is no way we can assign a dev_number to the
device which is not enumerated on the bus!
Am sure this is the same behavior with soundwire core too, atleast form
the code I can see it sets the assigned bit for only the devices that
are enumerated on the bus! Not all the devices specified in DT!
Unless I missed something!
--srini
--srini
