Re: [PATCH v2 00/13] soundwire: Add stream support

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 4/5/18 11:48 AM, Vinod Koul wrote:
> This series adds support in SoundWire subsystem for:
>   - Documentation for stream support
>   - stream management
>   - data port management
>   - DAI ops in cadence and Intel drivers
>   - ASoC API to propagate SDW stream
> 
> Updates in v2:
>    - Make ASoC API inlined
>    - Make stream states as states and not action
>    - Update the direction enum
>    - Fix some typos and comment updates

This is better than v1 and has few minor comments to be addressed but 
one big problem: you shot yourselves in the foot with the changes in 
state management that are inconsistent with the documentation (see e.g 
the patch #2). Without explanations on the intent that entire stream 
management section makes little sense. If you can fix this in a v3 the 
series will be in much better shape. Thanks!

> 
> Sanyog Kale (7):
>    soundwire: Add more documentation
>    soundwire: Add support for SoundWire stream management
>    soundwire: Add support for port management
>    soundwire: Add Master and Slave port programming
>    soundwire: Add helpers for ports operations
>    soundwire: Add bank switch routine
>    soundwire: Add stream configuration APIs
> 
> Shreyas NC (2):
>    ASoC: Add SoundWire stream programming interface
>    soundwire: Remove cdns_master_ops
> 
> Vinod Koul (4):
>    soundwire: cdns: Add port routines
>    soundwire: cdns: Add stream routines
>    soundwire: intel: Add stream initialization
>    soundwire: intel: Add audio DAI ops
> 
>   .../driver-api/soundwire/error_handling.rst        |   65 +
>   Documentation/driver-api/soundwire/index.rst       |    3 +
>   Documentation/driver-api/soundwire/locking.rst     |  106 ++
>   Documentation/driver-api/soundwire/stream.rst      |  368 +++++
>   drivers/soundwire/Kconfig                          |    2 +-
>   drivers/soundwire/Makefile                         |    2 +-
>   drivers/soundwire/bus.c                            |   43 +
>   drivers/soundwire/bus.h                            |   72 +
>   drivers/soundwire/cadence_master.c                 |  449 +++++-
>   drivers/soundwire/cadence_master.h                 |  151 ++
>   drivers/soundwire/intel.c                          |  528 ++++++-
>   drivers/soundwire/intel.h                          |    4 +
>   drivers/soundwire/intel_init.c                     |    3 +
>   drivers/soundwire/stream.c                         | 1549 ++++++++++++++++++++
>   include/linux/soundwire/sdw.h                      |  332 ++++-
>   include/linux/soundwire/sdw_intel.h                |   14 +
>   include/sound/soc-dai.h                            |   21 +
>   17 files changed, 3698 insertions(+), 14 deletions(-)
>   create mode 100644 Documentation/driver-api/soundwire/error_handling.rst
>   create mode 100644 Documentation/driver-api/soundwire/locking.rst
>   create mode 100644 Documentation/driver-api/soundwire/stream.rst
>   create mode 100644 drivers/soundwire/stream.c
>