All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Vinod Koul <vinod.koul@intel.com>, Takashi Iwai <tiwai@suse.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	ALSA <alsa-devel@alsa-project.org>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	Sudheer Papothi <spapothi@codeaurora.org>,
	plai@codeaurora.org, LKML <linux-kernel@vger.kernel.org>,
	patches.audio@intel.com, Mark <broonie@kernel.org>,
	srinivas.kandagatla@linaro.org, Shreyas NC <shreyas.nc@intel.com>,
	Sanyog Kale <sanyog.r.kale@intel.com>,
	Sagar Dharia <sdharia@codeaurora.org>,
	alan@linux.intel.com
Subject: Re: [alsa-devel] [PATCH 08/14] soundwire: Add Slave status handling helpers
Date: Wed, 1 Nov 2017 02:49:15 +0530	[thread overview]
Message-ID: <98f1b39a-b48d-2603-8e9d-88b4f804e2c8@linux.intel.com> (raw)
In-Reply-To: <20171031130412.GW3187@localhost>


> 
>>> +		if (found == false) {
>>> +			/* TODO: Park this device in Group 13 */
>>> +			dev_err(bus->dev, "Slave Entry not found");
>>
>> No break here?  Otherwise...
> 
> Thats intentional. We want to still read next device that show up
> 
>>
>>> +		}
>>> +
>>> +	} while (ret == 0);
>>
>> ... the outer loop may go endlessly.
>> This condition doesn't look effective.
> 
> not really. We cant keep reading successfully. At some point all slaves will
> ignore and return ENODATA and we exit. Bus errors will also make it exit
> 
> BUT given that we have seen stuff i am inclined to add a counter, we cant
> have more than 11 device so that's a sane value to use :)

Yep. Keep in mind however that there could be theoretical corner cases: 
if a device is enumerated, loses sync and becomes attached again while 
you deal with others, you'd have more than 11 iterations.

WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Vinod Koul <vinod.koul@intel.com>, Takashi Iwai <tiwai@suse.de>
Cc: ALSA <alsa-devel@alsa-project.org>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	patches.audio@intel.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	plai@codeaurora.org, LKML <linux-kernel@vger.kernel.org>,
	Sagar Dharia <sdharia@codeaurora.org>, Mark <broonie@kernel.org>,
	srinivas.kandagatla@linaro.org, Shreyas NC <shreyas.nc@intel.com>,
	Sanyog Kale <sanyog.r.kale@intel.com>,
	Sudheer Papothi <spapothi@codeaurora.org>,
	alan@linux.intel.com
Subject: Re: [PATCH 08/14] soundwire: Add Slave status handling helpers
Date: Wed, 1 Nov 2017 02:49:15 +0530	[thread overview]
Message-ID: <98f1b39a-b48d-2603-8e9d-88b4f804e2c8@linux.intel.com> (raw)
In-Reply-To: <20171031130412.GW3187@localhost>


> 
>>> +		if (found == false) {
>>> +			/* TODO: Park this device in Group 13 */
>>> +			dev_err(bus->dev, "Slave Entry not found");
>>
>> No break here?  Otherwise...
> 
> Thats intentional. We want to still read next device that show up
> 
>>
>>> +		}
>>> +
>>> +	} while (ret == 0);
>>
>> ... the outer loop may go endlessly.
>> This condition doesn't look effective.
> 
> not really. We cant keep reading successfully. At some point all slaves will
> ignore and return ENODATA and we exit. Bus errors will also make it exit
> 
> BUT given that we have seen stuff i am inclined to add a counter, we cant
> have more than 11 device so that's a sane value to use :)

Yep. Keep in mind however that there could be theoretical corner cases: 
if a device is enumerated, loses sync and becomes attached again while 
you deal with others, you'd have more than 11 iterations.

  reply	other threads:[~2017-10-31 21:19 UTC|newest]

Thread overview: 139+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19  3:03 [PATCH 00/14] soundwire: Add a new SoundWire subsystem Vinod Koul
2017-10-19  3:03 ` Vinod Koul
2017-10-19  3:03 ` [PATCH 01/14] Documentation: Add SoundWire summary Vinod Koul
2017-10-19  3:33   ` Randy Dunlap
2017-10-19  3:33     ` Randy Dunlap
2017-10-19  4:44     ` Vinod Koul
2017-10-19  4:44       ` Vinod Koul
2017-10-20 10:39   ` Greg Kroah-Hartman
2017-10-20 10:39     ` Greg Kroah-Hartman
2017-10-20 15:49     ` Vinod Koul
2017-10-20 15:49       ` Vinod Koul
2017-10-20 16:22       ` Greg Kroah-Hartman
2017-10-20 16:22         ` Greg Kroah-Hartman
2017-10-20 17:09         ` Vinod Koul
2017-10-20 17:09           ` Vinod Koul
2017-10-21  8:57   ` Mark Brown
2017-10-21  8:57     ` Mark Brown
2017-10-21 11:28     ` Vinod Koul
2017-10-21 11:28       ` Vinod Koul
2017-10-22 10:06       ` [alsa-devel] " Pierre-Louis Bossart
2017-10-23  8:21         ` Mark Brown
2017-10-23  7:50       ` Mark Brown
2017-10-23  7:50         ` Mark Brown
2017-10-23 11:18         ` [alsa-devel] " Vinod Koul
2017-10-19  3:03 ` [PATCH 02/14] soundwire: Add SoundWire bus type Vinod Koul
2017-10-19  7:40   ` Takashi Iwai
2017-10-19  8:32     ` [alsa-devel] " Takashi Iwai
2017-10-19  8:32       ` Takashi Iwai
2017-10-20  5:11     ` Vinod Koul
2017-10-20  5:11       ` Vinod Koul
2017-10-20  6:59       ` Takashi Iwai
2017-10-20 15:46         ` Vinod Koul
2017-10-20 15:50           ` Takashi Iwai
2017-10-20 15:50             ` Takashi Iwai
2017-10-20 16:11             ` Vinod Koul
2017-10-20 10:41   ` Greg Kroah-Hartman
2017-10-20 10:41     ` Greg Kroah-Hartman
2017-10-20 15:52     ` Vinod Koul
2017-10-20 10:45   ` Greg Kroah-Hartman
2017-10-20 16:01     ` Vinod Koul
2017-10-20 16:21       ` Greg Kroah-Hartman
2017-10-20 16:21         ` Greg Kroah-Hartman
2017-10-20 17:10         ` Vinod Koul
2017-10-23 11:46         ` Alan Cox
2017-10-23 11:46           ` Alan Cox
2017-10-26  8:33       ` Vinod Koul
2017-10-26  8:33         ` Vinod Koul
2017-10-27  8:57         ` Greg Kroah-Hartman
2017-10-30 13:11           ` Vinod Koul
2017-10-20 16:03     ` Philippe Ombredanne
2017-10-20 16:20       ` Vinod Koul
2017-10-20 16:20         ` Vinod Koul
2017-10-20 16:27         ` Greg Kroah-Hartman
2017-10-20 17:13           ` Vinod Koul
2017-10-20 17:13             ` Vinod Koul
2017-10-23 11:52           ` Alan Cox
2017-10-23 11:52             ` Alan Cox
2017-10-21  9:03   ` Mark Brown
2017-10-21 11:29     ` Vinod Koul
2017-10-21 11:29       ` Vinod Koul
2017-11-09 21:14   ` Srinivas Kandagatla
2017-11-09 21:14     ` Srinivas Kandagatla
2017-11-10  4:59     ` Vinod Koul
2017-11-10  4:59       ` Vinod Koul
2017-11-10  8:55       ` Vinod Koul
2017-11-10 10:50         ` Srinivas Kandagatla
2017-11-10 10:42       ` Srinivas Kandagatla
2017-11-10 10:42         ` Srinivas Kandagatla
2017-11-10 10:58         ` Vinod Koul
2017-10-19  3:03 ` [PATCH 03/14] soundwire: Add Master registration Vinod Koul
2017-10-19  3:03   ` Vinod Koul
2017-10-19  8:54   ` [alsa-devel] " Takashi Iwai
2017-10-19  8:54     ` Takashi Iwai
2017-10-20  5:19     ` [alsa-devel] " Vinod Koul
2017-10-20  5:19       ` Vinod Koul
2017-10-20 10:47   ` Greg Kroah-Hartman
2017-10-20 16:05     ` Vinod Koul
2017-10-21  9:12   ` Mark Brown
2017-10-21  9:12     ` Mark Brown
2017-10-21 11:35     ` Vinod Koul
2017-10-21 11:35       ` Vinod Koul
2017-10-23  8:24       ` Mark Brown
2017-10-23  8:24         ` Mark Brown
2017-10-23 11:19         ` Vinod Koul
2017-11-09 21:14   ` Srinivas Kandagatla
2017-11-10  5:02     ` Vinod Koul
2017-11-10  5:02       ` Vinod Koul
2017-10-19  3:03 ` [PATCH 04/14] soundwire: Add MIPI DisCo property helpers Vinod Koul
2017-10-19  9:02   ` [alsa-devel] " Takashi Iwai
2017-10-20  5:25     ` Vinod Koul
2017-10-21  9:20   ` Mark Brown
2017-10-21  9:20     ` Mark Brown
2017-10-21 11:37     ` Vinod Koul
2017-10-22 10:14       ` [alsa-devel] " Pierre-Louis Bossart
2017-10-22 10:14         ` Pierre-Louis Bossart
2017-10-19  3:03 ` [PATCH 05/14] soundwire: Add SoundWire MIPI defined registers Vinod Koul
2017-10-19  3:03 ` [PATCH 06/14] soundwire: Add IO transfer Vinod Koul
2017-10-19  3:03   ` Vinod Koul
2017-10-19  9:13   ` [alsa-devel] " Takashi Iwai
2017-10-20  5:30     ` Vinod Koul
2017-10-20  5:30       ` Vinod Koul
2017-10-20  7:06       ` [alsa-devel] " Takashi Iwai
2017-10-20  7:06         ` Takashi Iwai
2017-10-20 15:48         ` [alsa-devel] " Vinod Koul
2017-10-21  9:29   ` Mark Brown
2017-10-21  9:29     ` Mark Brown
2017-10-21 11:40     ` Vinod Koul
2017-10-19  3:03 ` [PATCH 07/14] regmap: Add SoundWire bus support Vinod Koul
2017-10-21  9:34   ` Mark Brown
2017-10-21  9:34     ` Mark Brown
2017-10-21 11:44     ` Vinod Koul
2017-10-23 11:56     ` Alan Cox
2017-10-23 13:16       ` Mark Brown
2017-10-19  3:03 ` [PATCH 08/14] soundwire: Add Slave status handling helpers Vinod Koul
2017-10-19 13:44   ` [alsa-devel] " Takashi Iwai
2017-10-31 13:04     ` Vinod Koul
2017-10-31 13:04       ` Vinod Koul
2017-10-31 21:19       ` Pierre-Louis Bossart [this message]
2017-10-31 21:19         ` Pierre-Louis Bossart
2017-11-01  9:08         ` [alsa-devel] " Vinod Koul
2017-11-01  9:08           ` Vinod Koul
2017-11-01 21:10           ` [alsa-devel] " Pierre-Louis Bossart
2017-11-02  3:28             ` Vinod Koul
2017-10-19  3:03 ` [PATCH 09/14] soundwire: Add slave status handling Vinod Koul
2017-10-19  3:03 ` [PATCH 10/14] soundwire: Add sysfs for SoundWire DisCo properties Vinod Koul
2017-10-21  9:42   ` Mark Brown
2017-10-21  9:42     ` Mark Brown
2017-10-21 11:53     ` Vinod Koul
2017-10-21 11:53       ` Vinod Koul
2017-11-09 21:14   ` Srinivas Kandagatla
2017-11-10  4:52     ` Vinod Koul
2017-10-19  3:03 ` [PATCH 11/14] soundwire: cdns: Add cadence module Vinod Koul
2017-10-21  9:52   ` Mark Brown
2017-10-21  9:52     ` Mark Brown
2017-10-21 11:54     ` Vinod Koul
2017-10-21 11:54       ` Vinod Koul
2017-10-19  3:03 ` [PATCH 12/14] soundwire: cdns: Add sdw_master_ops and IO transfer support Vinod Koul
2017-10-19  3:03 ` [PATCH 13/14] soundwire: intel: Add Intel Master driver Vinod Koul
2017-10-19  3:03 ` [PATCH 14/14] soundwire: intel: Add Intel init module 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=98f1b39a-b48d-2603-8e9d-88b4f804e2c8@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alan@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches.audio@intel.com \
    --cc=plai@codeaurora.org \
    --cc=sanyog.r.kale@intel.com \
    --cc=sdharia@codeaurora.org \
    --cc=shreyas.nc@intel.com \
    --cc=spapothi@codeaurora.org \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=tiwai@suse.de \
    --cc=vinod.koul@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.