lkml.org 
[lkml]   [2017]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [alsa-devel] [PATCH v4 08/15] soundwire: Add Slave status handling helpers
From
Date
On 12/3/17 11:08 AM, Vinod Koul wrote:
> On Fri, Dec 01, 2017 at 05:36:47PM -0600, Pierre-Louis Bossart wrote:
>
>>> +/* called with bus_lock held */
>>> +static int sdw_get_device_num(struct sdw_slave *slave)
>>> +{
>>> + int bit;
>>> +
>>> + bit = find_first_zero_bit(slave->bus->assigned, SDW_MAX_DEVICES);
>>> + if (bit == SDW_MAX_DEVICES) {
>>> + bit = -ENODEV;
>>> + goto err;
>>
>> My brain is starting to fry but is this correct? Bit11 seems like a valid
>> value. Should it be bit > 15 (assuming bit 12,13,14 are set to avoid using
>> groups and master)?
>
> this is correct. You are confusing SDW concept and API return types!
> That should be hint for you to start weekend if you didn't do so :D
>
> This API returns max value it was provided (last arg) if it doesn't
> find free bit. That's an indication to caller that we ran out of devices
> hence ENODEV error!

Can you just make sure bit11 is included?

>
>>> +static int sdw_program_device_num(struct sdw_bus *bus)
>>> +{
>>> + u8 buf[SDW_NUM_DEV_ID_REGISTERS] = {0};
>>> + struct sdw_slave *slave, *_s;
>>> + struct sdw_slave_id id;
>>> + struct sdw_msg msg;
>>> + bool found = false;
>>> + int count = 0, ret;
>>> + u64 addr;
>>> +
>>> + /* No Slave, so use raw xfer api */
>>> + ret = sdw_fill_msg(&msg, NULL, SDW_SCP_DEVID_0,
>>> + SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + do {
>>> + ret = sdw_transfer(bus, NULL, &msg);
>>> + if (ret == -ENODATA) { /* end of device id reads */
>>> + ret = 0;
>>> + break;
>>> + }
>>> + if (ret < 0) {
>>> + dev_err(bus->dev, "DEVID read fail:%d\n", ret);
>>> + break;
>>> + }
>>> +
>>> + /*
>>> + * Construct the addr and extract. Cast the higher shift
>>> + * bits to avoid truncation due to size limit.
>>> + */
>>> + addr = buf[5] | (buf[4] << 8) | (buf[3] << 16) |
>>> + (buf[2] << 24) | ((unsigned long long)buf[1] << 32) |
>>> + ((unsigned long long)buf[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) {
>>> + found = true;
>>> +
>>> + /*
>>> + * Assign a new dev_num to this Slave and
>>> + * not mark it present. It will be marked
>>> + * present after it reports ATTACHED on new
>>> + * dev_num
>>> + */
>>> + ret = sdw_assign_device_num(slave);
>>> + if (ret) {
>>> + dev_err(slave->bus->dev,
>>> + "Assign dev_num failed:%d",
>>> + ret);
>>> + return ret;
>>> + }
>>> +
>>> + break;
>>> + }
>>> + }
>>> +
>>> + if (found == false) {
>>> + /* TODO: Park this device in Group 13 */
>>> + dev_err(bus->dev, "Slave Entry not found");
>>> + }
>>> +
>>> + count++;
>>> +
>>> + } while (ret == 0 && count < (SDW_MAX_DEVICES * 2));
>>
>> explain that the last condition is intentional - this is not a bug -, some
>> devices can drop off during enumeration and rejoin so might be counted
>> twice.
>
> ok will add
>

\
 
 \ /
  Last update: 2017-12-04 04:07    [W:0.188 / U:0.912 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site