From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pierre-Louis Bossart Subject: Re: [PATCH v2 01/13] soundwire: Add more documentation Date: Fri, 6 Apr 2018 10:24:08 -0500 Message-ID: References: <1522946904-2089-1-git-send-email-vinod.koul@intel.com> <1522946904-2089-2-git-send-email-vinod.koul@intel.com> <1f54fc62-8da0-ee1e-327c-30230b5f00f6@linux.intel.com> <20180406032449.GD6014@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by alsa0.perex.cz (Postfix) with ESMTP id 757BD266BBC for ; Fri, 6 Apr 2018 17:24:11 +0200 (CEST) In-Reply-To: <20180406032449.GD6014@localhost> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Vinod Koul Cc: ALSA , tiwai@suse.de, Greg KH , liam.r.girdwood@linux.intel.com, patches.audio@intel.com, broonie@kernel.org, Sanyog Kale List-Id: alsa-devel@alsa-project.org On 4/5/18 10:24 PM, Vinod Koul wrote: > On Thu, Apr 05, 2018 at 04:37:14PM -0500, Pierre-Louis Bossart wrote: >> On 4/5/18 11:48 AM, Vinod Koul wrote: > >>> +SoundWire stream states >>> +----------------------- >>> + >>> +Below shows the SoundWire stream states and state transition diagram. :: >>> + >>> + +-----------+ +------------+ +----------+ +----------+ >>> + | ALLOCATED +---->| CONFIGURED +---->| PREPARED +---->| ENABLED | >>> + | STATE | | STATE | | STATE | | STATE | >>> + +-----------+ +------------+ +----------+ +----+-----+ >>> + ^ >>> + | >>> + | >>> + v >>> + +----------+ +------------+ +----+-----+ >>> + | RELEASED |<----------+ DEPREPARED |<-------+ DISABLED | >>> + | STATE | | STATE | | STATE | >>> + +----------+ +------------+ +----------+ >>> + >> >> Patch 2 describes the states as below: >> >> +enum sdw_stream_state { >> + SDW_STREAM_RELEASED = 0, >> + SDW_STREAM_ALLOCATED = 1, >> + SDW_STREAM_CONFIGURED = 2, >> + SDW_STREAM_PREPARED = 3, >> + SDW_STREAM_ENABLED = 4, >> + SDW_STREAM_DISABLED = 5, >> + SDW_STREAM_DEPREPARED = 6, >> >> which isn't the same picture as the ascii art above. The RELEASED state is >> the starting point, and there's an arrow missing from RELEASED to ALLOCATED. > > The enumerator describes the values given for each state. That has no > relation whatsoever to state transitions allowed. I don't recall why people > made SDW_STREAM_RELEASED as Zero, it could very well have been 7 and nothing > changes in diagram. > > Now in the last review I did mention to you that there is NO transition > between RELEASED and ALLOCATED, hence no arrow can be made. > > Your point on initial state is not right. I can we go ahead and do a > kmalloc() instead of current kzalloc() for the structure which would make > stream state from garbage/uninitialized to ALLOCATED and eventually finally > into RELEASED and it being freed. See no move between ALLOCATED and > RELEASED! since RELEASED is the value zero, it is the implicit start state, like it or not. If you don't initialize your structure then your tests may fail or your stream might be allocated when it's not. It's just engineering 101 to define a state machine with a known start point...