[alsa-devel] [PATCH v2 01/13] soundwire: Add more documentation

[Pierre-Louis Bossart]

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...