mbox series

[RFC,0/2] soundwire: add master_device/driver support

Message ID 20200201042011.5781-1-pierre-louis.bossart@linux.intel.com (mailing list archive)
Headers show
Series soundwire: add master_device/driver support | expand

Message

Pierre-Louis Bossart Feb. 1, 2020, 4:20 a.m. UTC
The SoundWire master representation needs to evolve to take into account:

a) a May 2019 recommendation from Greg KH to remove platform devices

b) the need on Intel platforms to support hardware startup only once
the power rail dependencies are settled. The SoundWire master is not
an independent IP at all.

c) the need to deal with external wakes handled by the PCI subsystem
in specific low-power modes

In case it wasn't clear, the SoundWire subsystem is currently unusable
with v5.5 on devices hitting the shelves very soon (race conditions,
power management, suspend/resume, etc). v5.6 will only provide
interface changes and no functional improvement. We've circled on the
same concepts since September 2019, and I hope this direction is now
aligned with the suggestions from Vinod Koul and that we can target
v5.7 as the 'SoundWire just works(tm)' version.

This series is provided as an RFC since it depends on patches already
for review.

Pierre-Louis Bossart (2):
  soundwire: bus_type: add master_device/driver support
  soundwire: intel: transition to sdw_master_device/driver support

 drivers/soundwire/Makefile         |   2 +-
 drivers/soundwire/bus_type.c       | 141 +++++++++++++-
 drivers/soundwire/intel.c          |  96 ++++++----
 drivers/soundwire/intel.h          |   8 +-
 drivers/soundwire/intel_init.c     | 283 ++++++++++++++++++++++-------
 drivers/soundwire/master.c         | 119 ++++++++++++
 drivers/soundwire/slave.c          |   7 +-
 include/linux/soundwire/sdw.h      |  83 +++++++++
 include/linux/soundwire/sdw_type.h |  36 +++-
 9 files changed, 662 insertions(+), 113 deletions(-)
 create mode 100644 drivers/soundwire/master.c

Comments

Greg Kroah-Hartman Feb. 14, 2020, 4:49 p.m. UTC | #1
On Fri, Jan 31, 2020 at 10:20:09PM -0600, Pierre-Louis Bossart wrote:
> The SoundWire master representation needs to evolve to take into account:
> 
> a) a May 2019 recommendation from Greg KH to remove platform devices
> 
> b) the need on Intel platforms to support hardware startup only once
> the power rail dependencies are settled. The SoundWire master is not
> an independent IP at all.
> 
> c) the need to deal with external wakes handled by the PCI subsystem
> in specific low-power modes
> 
> In case it wasn't clear, the SoundWire subsystem is currently unusable
> with v5.5 on devices hitting the shelves very soon (race conditions,
> power management, suspend/resume, etc). v5.6 will only provide
> interface changes and no functional improvement. We've circled on the
> same concepts since September 2019, and I hope this direction is now
> aligned with the suggestions from Vinod Koul and that we can target
> v5.7 as the 'SoundWire just works(tm)' version.
> 
> This series is provided as an RFC since it depends on patches already
> for review.

Both of these look sane to me, nice work!

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Pierre-Louis Bossart Feb. 14, 2020, 11:34 p.m. UTC | #2
On 2/14/20 10:49 AM, Greg KH wrote:
> On Fri, Jan 31, 2020 at 10:20:09PM -0600, Pierre-Louis Bossart wrote:
>> The SoundWire master representation needs to evolve to take into account:
>>
>> a) a May 2019 recommendation from Greg KH to remove platform devices
>>
>> b) the need on Intel platforms to support hardware startup only once
>> the power rail dependencies are settled. The SoundWire master is not
>> an independent IP at all.
>>
>> c) the need to deal with external wakes handled by the PCI subsystem
>> in specific low-power modes
>>
>> In case it wasn't clear, the SoundWire subsystem is currently unusable
>> with v5.5 on devices hitting the shelves very soon (race conditions,
>> power management, suspend/resume, etc). v5.6 will only provide
>> interface changes and no functional improvement. We've circled on the
>> same concepts since September 2019, and I hope this direction is now
>> aligned with the suggestions from Vinod Koul and that we can target
>> v5.7 as the 'SoundWire just works(tm)' version.
>>
>> This series is provided as an RFC since it depends on patches already
>> for review.
> 
> Both of these look sane to me, nice work!
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


Thanks Greg, I really appreciate the review and educational guidance on 
the use of platform devices!

If that's alright with Vinod and you, I'd like to defer the integration 
of these patches a bit so that we can improve them a bit e.g.

- I realized earlier this week there's a potential issue with the 
'shutdown' callback, what I did looks duplicated with 'shutdown' called 
from the device core and by the parent.

- it would make sense to add support for this 'master device' in the 
Qualcomm drivers as well, so that we are aligned with a similar device 
hierarchy (PCI -> sdw_master_device and DT/plaforms ->sdw_master_device).

- we probably want to add the sysfs patches that started this discussion 
on the platform devices. This would be very useful to debug new 
platforms, we depend on BIOS/firmware and we can't always know the 
actual settings just by looking at the DSDT static contents (multiple 
tests and overridden values).

My preference in terms of integration in drivers/soundwire would be

1. Intel DAI cleanup (only one kfree missing, will resubmit v3 today)

2. [PATCH 00/10] soundwire: bus: fix race conditions, add suspend-resume
this series solves a lot of issues and should go first.

3. ASoC/SOF integration (still with platform devices)
I narrowed this down to 6 patches, that would help me submit the rest of 
the ASoC/SOF patches in Mark Brown's tree. That would be Intel specific. 
This step would be the first where everything SoundWire-related can be 
enabled in a build, and while we've caught a lot of cross-compilation 
issues it's likely some bots will find corner cases to keep us busy.

4. master_device/driver transition: these updated patches removing 
platform devices + sysfs support + Qualcomm support (the last point 
would depend on the workload/support of Qualcomm/Linaro folks, I don't 
want to commit on their behalf).

5. New SoundWire functionality for Intel platforms (clock-stop/restart 
and synchronized links). The code would be only located in 
drivers/soundwire and be easier to handle. For the synchronized links we 
still have a bit of validation work to do so it should really come last.

Would this work for everyone?

Thanks,
-Pierre
Greg Kroah-Hartman Feb. 15, 2020, 12:05 a.m. UTC | #3
On Fri, Feb 14, 2020 at 05:34:40PM -0600, Pierre-Louis Bossart wrote:
> 
> My preference in terms of integration in drivers/soundwire would be
> 
> 1. Intel DAI cleanup (only one kfree missing, will resubmit v3 today)
> 
> 2. [PATCH 00/10] soundwire: bus: fix race conditions, add suspend-resume
> this series solves a lot of issues and should go first.
> 
> 3. ASoC/SOF integration (still with platform devices)
> I narrowed this down to 6 patches, that would help me submit the rest of the
> ASoC/SOF patches in Mark Brown's tree. That would be Intel specific. This
> step would be the first where everything SoundWire-related can be enabled in
> a build, and while we've caught a lot of cross-compilation issues it's
> likely some bots will find corner cases to keep us busy.
> 
> 4. master_device/driver transition: these updated patches removing platform
> devices + sysfs support + Qualcomm support (the last point would depend on
> the workload/support of Qualcomm/Linaro folks, I don't want to commit on
> their behalf).
> 
> 5. New SoundWire functionality for Intel platforms (clock-stop/restart and
> synchronized links). The code would be only located in drivers/soundwire and
> be easier to handle. For the synchronized links we still have a bit of
> validation work to do so it should really come last.
> 
> Would this work for everyone?

Sounds reasonable to me, but patches would show it best to see if there
are any issues :)

thanks,

greg k-h