linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support
@ 2020-11-13 16:18 Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 01/10] Add auxiliary bus support Dave Ertman
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

Brief history of Auxiliary Bus
==============================
The auxiliary bus code was originally submitted upstream as virtual
bus, and was submitted through the netdev tree.  This process generated
up to v4.  This discussion can be found here:
https://lore.kernel.org/netdev/20191111192219.30259-1-jeffrey.t.kirsher@intel.com/#t

At this point, GregKH requested that we take the review and revision
process to an internal mailing list and garner the buy-in of a respected
kernel contributor.

The auxiliary bus (then known as virtual bus) was originally submitted
along with implementation code for the ice driver and irdma drive,
causing the complication of also having dependencies in the rdma tree.
This new submission is utilizing an auxiliary bus consumer in only the
sound driver tree to create the initial implementation and a single
user.

Since implementation work has started on this patch set, there have been
multiple inquiries about the time frame of its completion.  It appears
that there will be numerous consumers of this functionality.

The process of internal review and implementation using the sound
drivers generated 19 internal versions.  The changes, including the name
change from virtual bus to auxiliary bus, from these versions can be
summarized as the following:

- Fixed compilation and checkpatch errors
- Improved documentation to address the motivation for virtual bus.
- Renamed virtual bus to auxiliary bus
- increased maximum device name size
- Correct order in Kconfig and Makefile
- removed the mid-layer adev->release layer for device unregister
- pushed adev->id management to parent driver
- all error paths out of ancillary_device_register return error code
- all error paths out of ancillary_device_register use put_device
- added adev->name element
- modname in register cannot be NULL
- added KBUILD_MODNAME as prefix for match_name
- push adev->id responsibility to registering driver
- uevent now parses adev->dev name
- match_id function now parses adev->dev name
- changed drivers probe function to also take an ancillary_device_id
  param
- split ancillary_device_register into device_initialize and device_add
- adjusted what is done in device_initialize and device_add
- change adev to ancildev and adrv to ancildrv
- change adev to ancildev in documentation

==========================

Introduces the auxiliary bus implementation along with the example usage
in the Sound Open Firmware(SOF) audio driver.

In some subsystems, the functionality of the core device
(PCI/ACPI/other) may be too complex for a single device to be managed as
a monolithic block or a part of the functionality might need to be
exposed to a different subsystem.  Splitting the functionality into
smaller orthogonal devices makes it easier to manage data, power
management and domain-specific communication with the hardware.  Also,
common auxiliary_device functionality across primary devices can be
handled by a common auxiliary_device. A key requirement for such a split
is that there is no dependency on a physical bus, device, register
accesses or regmap support. These individual devices split from the core
cannot live on the platform bus as they are not physical devices that
are controlled by DT/ACPI. The same argument applies for not using MFD
in this scenario as it relies on individual function devices being
physical devices that are DT enumerated.

An example for this kind of requirement is the audio subsystem where a
single IP handles multiple entities such as HDMI, Soundwire, local
devices such as mics/speakers etc. The split for the core's
functionality can be arbitrary or be defined by the DSP firmware
topology and include hooks for test/debug. This allows for the audio
core device to be minimal and tightly coupled with handling the
hardware-specific logic and communication.

The auxiliary bus is intended to be minimal, generic and avoid
domain-specific assumptions. Each auxiliary bus device represents a part
of its parent functionality. The generic behavior can be extended and
specialized as needed by encapsulating an auxiliary bus device within
other domain-specific structures and the use of .ops callbacks.

The SOF driver adopts the auxiliary bus for implementing the
multi-client support. A client in the context of the SOF driver
represents a part of the core device's functionality. It is not a
physical device but rather an auxiliary device that needs to communicate
with the DSP via IPCs. With multi-client support,the sound card can be
separated into multiple orthogonal auxiliary devices for local devices
(mic/speakers etc), HDMI, sensing, probes, debug etc.  In this series,
we demonstrate the usage of the auxiliary bus with the help of the IPC
test client which is used for testing the serialization of IPCs when
multiple clients talk to the DSP at the same time.

v4 changes:
clean up passive voice
clarify reg/unreg path for devices
explicitly call out GPL v2

v3 changes:
rename to auxiliary bus
move .c file to drivers/base/
split auxdev unregister flow into uninitialize and delete steps
update kernel-doc on register functions for new unregister model
update documentation with new name and unregister flow
remove check for release in auxiliary bus, allow core to catch
Change driver register so only probe and id_table mandatory
Fix matching logic in auxillary_match_id to account for longer id->name
utilize auxiliary_bus callbacks for probe, remove and shutdown
add auxiliary_find_device function
add code to ensure unique auxdrv name
shorten initialize/uninitialize in function names to init/uninit
simplify looping logic in match_id
in drives/base/Kconfig changed from tristate to bool
Modified signature SOF client register/unregister API
Modified PM runtime enabling sequence in the SOF ipc test aux driver
Removed driver.name from the aux driver
Added Probes client aux driver and device registration in the
        SOF driver. This allows for enabling the probes functionality in
the SOF
        firmware for audio data extraction from specific points in the
audio
        pipeline. Without auxiliary bus, the implementation requires
modifying
        existing 15+ machine drivers to add the probes DAI links to the
sounds
        cards. Using the auxiliary bus allows for splitting the probes
        implementation from the SOF core making it easy to maintain.

v2 changes:
defined pr_fmt for kernel messages
replaced WARN_ON calls in registration with pr_err calls
adding kernel-doc function comments for device_initialize and device_add
fix typo in documentation
removed inaccurate line in documentation
fixed formatting in drivers/bus/Makefile
changed unwind path for sof_client_dev_alloc()
improved comments for client list and mem freeing during client unreg
removed debugfs entries in sof_ipc_test_client_drv during remove
changed the signature of sof_debug_ipc_flood_test()
fix a looping error in ancillary_match_id
updated error value in sof_client_dev_register()
mutex held while traversing client list when unregistering clients
updated includes in sof-client.h

Dave Ertman (1):
  Add auxiliary bus support

Ranjani Sridharan (9):
  ASoC: SOF: Introduce descriptors for SOF client
  ASoC: SOF: Create client driver for IPC test
  ASoC: SOF: ops: Add ops for client registration
  ASoC: SOF: Intel: Define ops for client registration
  ASoC: SOF: Intel: Remove IPC flood test support in SOF core
  ASoC: SOF: sof-client: Add client APIs to access probes ops
  ASoC: SOF: compress: move and export sof_probe_compr_ops
  ASoC: SOF: Add new client driver for probes support
  ASoC: SOF: Intel: CNL: register probes client

 Documentation/driver-api/auxiliary_bus.rst | 234 +++++++++++
 Documentation/driver-api/index.rst         |   1 +
 drivers/base/Kconfig                       |   3 +
 drivers/base/Makefile                      |   1 +
 drivers/base/auxiliary.c                   | 268 ++++++++++++
 include/linux/auxiliary_bus.h              |  78 ++++
 include/linux/mod_devicetable.h            |   8 +
 scripts/mod/devicetable-offsets.c          |   3 +
 scripts/mod/file2alias.c                   |   8 +
 sound/soc/sof/Kconfig                      |  47 ++-
 sound/soc/sof/Makefile                     |  10 +-
 sound/soc/sof/compress.c                   |  60 +--
 sound/soc/sof/compress.h                   |   1 +
 sound/soc/sof/core.c                       |  18 +-
 sound/soc/sof/debug.c                      | 457 ---------------------
 sound/soc/sof/intel/Kconfig                |   9 +
 sound/soc/sof/intel/Makefile               |   3 +
 sound/soc/sof/intel/apl.c                  |  16 +
 sound/soc/sof/intel/bdw.c                  |  16 +
 sound/soc/sof/intel/byt.c                  |  20 +
 sound/soc/sof/intel/cnl.c                  |  32 ++
 sound/soc/sof/intel/hda-dai.c              |  27 --
 sound/soc/sof/intel/hda.h                  |   6 -
 sound/soc/sof/intel/intel-client.c         |  40 ++
 sound/soc/sof/intel/intel-client.h         |  26 ++
 sound/soc/sof/ops.h                        |  14 +
 sound/soc/sof/pcm.c                        |  11 -
 sound/soc/sof/probe.c                      | 124 +++---
 sound/soc/sof/probe.h                      |  41 +-
 sound/soc/sof/sof-client.c                 | 170 ++++++++
 sound/soc/sof/sof-client.h                 |  91 ++++
 sound/soc/sof/sof-ipc-test-client.c        | 321 +++++++++++++++
 sound/soc/sof/sof-priv.h                   |  23 +-
 sound/soc/sof/sof-probes-client.c          | 414 +++++++++++++++++++
 34 files changed, 1975 insertions(+), 626 deletions(-)
 create mode 100644 Documentation/driver-api/auxiliary_bus.rst
 create mode 100644 drivers/base/auxiliary.c
 create mode 100644 include/linux/auxiliary_bus.h
 create mode 100644 sound/soc/sof/intel/intel-client.c
 create mode 100644 sound/soc/sof/intel/intel-client.h
 create mode 100644 sound/soc/sof/sof-client.c
 create mode 100644 sound/soc/sof/sof-client.h
 create mode 100644 sound/soc/sof/sof-ipc-test-client.c
 create mode 100644 sound/soc/sof/sof-probes-client.c

-- 
2.26.2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 01/10] Add auxiliary bus support
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-17  5:30   ` Leon Romanovsky
  2020-11-17 19:39   ` Martin Habets
  2020-11-13 16:18 ` [PATCH v4 02/10] ASoC: SOF: Introduce descriptors for SOF client Dave Ertman
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
It enables drivers to create an auxiliary_device and bind an
auxiliary_driver to it.

The bus supports probe/remove shutdown and suspend/resume callbacks.
Each auxiliary_device has a unique string based id; driver binds to
an auxiliary_device based on this id through the bus.

Co-developed-by: Kiran Patil <kiran.patil@intel.com>
Signed-off-by: Kiran Patil <kiran.patil@intel.com>
Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
Reviewed-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 Documentation/driver-api/auxiliary_bus.rst | 234 ++++++++++++++++++
 Documentation/driver-api/index.rst         |   1 +
 drivers/base/Kconfig                       |   3 +
 drivers/base/Makefile                      |   1 +
 drivers/base/auxiliary.c                   | 268 +++++++++++++++++++++
 include/linux/auxiliary_bus.h              |  78 ++++++
 include/linux/mod_devicetable.h            |   8 +
 scripts/mod/devicetable-offsets.c          |   3 +
 scripts/mod/file2alias.c                   |   8 +
 9 files changed, 604 insertions(+)
 create mode 100644 Documentation/driver-api/auxiliary_bus.rst
 create mode 100644 drivers/base/auxiliary.c
 create mode 100644 include/linux/auxiliary_bus.h

diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
new file mode 100644
index 000000000000..5dd7804631ef
--- /dev/null
+++ b/Documentation/driver-api/auxiliary_bus.rst
@@ -0,0 +1,234 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+=============
+Auxiliary Bus
+=============
+
+In some subsystems, the functionality of the core device (PCI/ACPI/other) is
+too complex for a single device to be managed by a monolithic driver
+(e.g. Sound Open Firmware), multiple devices might implement a common
+intersection of functionality (e.g. NICs + RDMA), or a driver may want to
+export an interface for another subsystem to drive (e.g. SIOV Physical Function
+export Virtual Function management).  A split of the functinoality into child-
+devices representing sub-domains of functionality makes it possible to
+compartmentalize, layer, and distribute domain-specific concerns via a Linux
+device-driver model.
+
+An example for this kind of requirement is the audio subsystem where a single
+IP is handling multiple entities such as HDMI, Soundwire, local devices such as
+mics/speakers etc. The split for the core's functionality can be arbitrary or
+be defined by the DSP firmware topology and include hooks for test/debug. This
+allows for the audio core device to be minimal and focused on hardware-specific
+control and communication.
+
+Each auxiliary_device represents a part of its parent functionality. The
+generic behavior can be extended and specialized as needed by encapsulating an
+auxiliary_device within other domain-specific structures and the use of .ops
+callbacks. Devices on the auxiliary bus do not share any structures and the use
+of a communication channel with the parent is domain-specific.
+
+Note that ops are intended as a way to augment instance behavior within a class
+of auxiliary devices, it is not the mechanism for exporting common
+infrastructure from the parent. Consider EXPORT_SYMBOL_NS() to convey
+infrastructure from the parent module to the auxiliary module(s).
+
+
+When Should the Auxiliary Bus Be Used
+=====================================
+
+The auxiliary bus is to be used when a driver and one or more kernel modules,
+who share a common header file with the driver, need a mechanism to connect and
+provide access to a shared object allocated by the auxiliary_device's
+registering driver.  The registering driver for the auxiliary_device(s) and the
+kernel module(s) registering auxiliary_drivers can be from the same subsystem,
+or from multiple subsystems.
+
+The emphasis here is on a common generic interface that keeps subsystem
+customization out of the bus infrastructure.
+
+One example is a PCI network device that is RDMA-capable and exports a child
+device to be driven by an auxiliary_driver in the RDMA subsystem.  The PCI
+driver allocates and registers an auxiliary_device for each physical
+function on the NIC.  The RDMA driver registers an auxiliary_driver that claims
+each of these auxiliary_devices.  This conveys data/ops published by the parent
+PCI device/driver to the RDMA auxiliary_driver.
+
+Another use case is for the PCI device to be split out into multiple sub
+functions.  For each sub function an auxiliary_device is created.  A PCI sub
+function driver binds to such devices that creates its own one or more class
+devices.  A PCI sub function auxiliary device is likely to be contained in a
+struct with additional attributes such as user defined sub function number and
+optional attributes such as resources and a link to the parent device.  These
+attributes could be used by systemd/udev; and hence should be initialized
+before a driver binds to an auxiliary_device.
+
+A key requirement for utilizing the auxiliary bus is that there is no
+dependency on a physical bus, device, register accesses or regmap support.
+These individual devices split from the core cannot live on the platform bus as
+they are not physical devices that are controlled by DT/ACPI.  The same
+argument applies for not using MFD in this scenario as MFD relies on individual
+function devices being physical devices.
+
+Auxiliary Device
+================
+
+An auxiliary_device represents a part of its parent device's functionality. It
+is given a name that, combined with the registering drivers KBUILD_MODNAME,
+creates a match_name that is used for driver binding, and an id that combined
+with the match_name provide a unique name to register with the bus subsystem.
+
+Registering an auxiliary_device is a two-step process.  First call
+auxiliary_device_init(), which checks several aspects of the auxiliary_device
+struct and performs a device_initialize().  After this step completes, any
+error state must have a call to auxiliary_device_uninit() in its resolution path.
+The second step in registering an auxiliary_device is to perform a call to
+auxiliary_device_add(), which sets the name of the device and add the device to
+the bus.
+
+Unregistering an auxiliary_device is also a two-step process to mirror the
+register process.  First call auxiliary_device_delete(), then call
+auxiliary_device_uninit().
+
+.. code-block:: c
+
+	struct auxiliary_device {
+		struct device dev;
+                const char *name;
+		u32 id;
+	};
+
+If two auxiliary_devices both with a match_name "mod.foo" are registered onto
+the bus, they must have unique id values (e.g. "x" and "y") so that the
+registered devices names are "mod.foo.x" and "mod.foo.y".  If match_name + id
+are not unique, then the device_add fails and generates an error message.
+
+The auxiliary_device.dev.type.release or auxiliary_device.dev.release must be
+populated with a non-NULL pointer to successfully register the auxiliary_device.
+
+The auxiliary_device.dev.parent must also be populated.
+
+Auxiliary Device Memory Model and Lifespan
+------------------------------------------
+
+The registering driver is the entity that allocates memory for the
+auxiliary_device and register it on the auxiliary bus.  It is important to note
+that, as opposed to the platform bus, the registering driver is wholly
+responsible for the management for the memory used for the driver object.
+
+A parent object, defined in the shared header file, contains the
+auxiliary_device.  It also contains a pointer to the shared object(s), which
+also is defined in the shared header.  Both the parent object and the shared
+object(s) are allocated by the registering driver.  This layout allows the
+auxiliary_driver's registering module to perform a container_of() call to go
+from the pointer to the auxiliary_device, that is passed during the call to the
+auxiliary_driver's probe function, up to the parent object, and then have
+access to the shared object(s).
+
+The memory for the auxiliary_device is freed only in its release() callback
+flow as defined by its registering driver.
+
+The memory for the shared object(s) must have a lifespan equal to, or greater
+than, the lifespan of the memory for the auxiliary_device.  The auxiliary_driver
+should only consider that this shared object is valid as long as the
+auxiliary_device is still registered on the auxiliary bus.  It is up to the
+registering driver to manage (e.g. free or keep available) the memory for the
+shared object beyond the life of the auxiliary_device.
+
+The registering driver must unregister all auxiliary devices before its own
+driver.remove() is completed.
+
+Auxiliary Drivers
+=================
+
+Auxiliary drivers follow the standard driver model convention, where
+discovery/enumeration is handled by the core, and drivers
+provide probe() and remove() methods. They support power management
+and shutdown notifications using the standard conventions.
+
+.. code-block:: c
+
+	struct auxiliary_driver {
+		int (*probe)(struct auxiliary_device *,
+                             const struct auxiliary_device_id *id);
+		int (*remove)(struct auxiliary_device *);
+		void (*shutdown)(struct auxiliary_device *);
+		int (*suspend)(struct auxiliary_device *, pm_message_t);
+		int (*resume)(struct auxiliary_device *);
+		struct device_driver driver;
+		const struct auxiliary_device_id *id_table;
+	};
+
+Auxiliary drivers register themselves with the bus by calling
+auxiliary_driver_register(). The id_table contains the match_names of auxiliary
+devices that a driver can bind with.
+
+Example Usage
+=============
+
+Auxiliary devices are created and registered by a subsystem-level core device
+that needs to break up its functionality into smaller fragments. One way to
+extend the scope of an auxiliary_device is to encapsulate it within a domain-
+pecific structure defined by the parent device. This structure contains the
+auxiliary_device and any associated shared data/callbacks needed to establish
+the connection with the parent.
+
+An example is:
+
+.. code-block:: c
+
+        struct foo {
+		struct auxiliary_device auxdev;
+		void (*connect)(struct auxiliary_device *auxdev);
+		void (*disconnect)(struct auxiliary_device *auxdev);
+		void *data;
+        };
+
+The parent device then registers the auxiliary_device by calling
+auxiliary_device_init(), and then auxiliary_device_add(), with the pointer to
+the auxdev member of the above structure. The parent provides a name for the
+auxiliary_device that, combined with the parent's KBUILD_MODNAME, creates a
+match_name that is be used for matching and binding with a driver.
+
+Whenever an auxiliary_driver is registered, based on the match_name, the
+auxiliary_driver's probe() is invoked for the matching devices.  The
+auxiliary_driver can also be encapsulated inside custom drivers that make the
+core device's functionality extensible by adding additional domain-specific ops
+as follows:
+
+.. code-block:: c
+
+	struct my_ops {
+		void (*send)(struct auxiliary_device *auxdev);
+		void (*receive)(struct auxiliary_device *auxdev);
+	};
+
+
+	struct my_driver {
+		struct auxiliary_driver auxiliary_drv;
+		const struct my_ops ops;
+	};
+
+An example of this type of usage is:
+
+.. code-block:: c
+
+	const struct auxiliary_device_id my_auxiliary_id_table[] = {
+		{ .name = "foo_mod.foo_dev" },
+		{ },
+	};
+
+	const struct my_ops my_custom_ops = {
+		.send = my_tx,
+		.receive = my_rx,
+	};
+
+	const struct my_driver my_drv = {
+		.auxiliary_drv = {
+			.name = "myauxiliarydrv",
+			.id_table = my_auxiliary_id_table,
+			.probe = my_probe,
+			.remove = my_remove,
+			.shutdown = my_shutdown,
+		},
+		.ops = my_custom_ops,
+	};
diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index f357f3eb400c..86759a74b7f1 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -72,6 +72,7 @@ available subsections can be seen below.
    thermal/index
    fpga/index
    acpi/index
+   auxiliary_bus
    backlight/lp855x-driver.rst
    connector
    console
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 8d7001712062..040be48ce046 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -1,6 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 menu "Generic Driver Options"
 
+config AUXILIARY_BUS
+	bool
+
 config UEVENT_HELPER
 	bool "Support for uevent helper"
 	help
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 41369fc7004f..5e7bf9669a81 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -7,6 +7,7 @@ obj-y			:= component.o core.o bus.o dd.o syscore.o \
 			   attribute_container.o transport_class.o \
 			   topology.o container.o property.o cacheinfo.o \
 			   swnode.o
+obj-$(CONFIG_AUXILIARY_BUS) += auxiliary.o
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
 obj-y			+= power/
 obj-$(CONFIG_ISA_BUS_API)	+= isa.o
diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
new file mode 100644
index 000000000000..ef2af417438b
--- /dev/null
+++ b/drivers/base/auxiliary.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2019-2020 Intel Corporation
+ *
+ * Please see Documentation/driver-api/auxiliary_bus.rst for more information.
+ */
+
+#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/string.h>
+#include <linux/auxiliary_bus.h>
+
+static const struct auxiliary_device_id *auxiliary_match_id(const struct auxiliary_device_id *id,
+							    const struct auxiliary_device *auxdev)
+{
+	for (; id->name[0]; id++) {
+		const char *p = strrchr(dev_name(&auxdev->dev), '.');
+		int match_size;
+
+		if (!p)
+			continue;
+		match_size = p - dev_name(&auxdev->dev);
+
+		/* use dev_name(&auxdev->dev) prefix before last '.' char to match to */
+		if (strlen(id->name) == match_size &&
+		    !strncmp(dev_name(&auxdev->dev), id->name, match_size))
+			return id;
+	}
+	return NULL;
+}
+
+static int auxiliary_match(struct device *dev, struct device_driver *drv)
+{
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+	struct auxiliary_driver *auxdrv = to_auxiliary_drv(drv);
+
+	return !!auxiliary_match_id(auxdrv->id_table, auxdev);
+}
+
+static int auxiliary_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	const char *name, *p;
+
+	name = dev_name(dev);
+	p = strrchr(name, '.');
+
+	return add_uevent_var(env, "MODALIAS=%s%.*s", AUXILIARY_MODULE_PREFIX, (int)(p - name),
+			      name);
+}
+
+static const struct dev_pm_ops auxiliary_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(pm_generic_runtime_suspend, pm_generic_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
+};
+
+static int auxiliary_bus_probe(struct device *dev)
+{
+	struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+	int ret;
+
+	ret = dev_pm_domain_attach(dev, true);
+	if (ret) {
+		dev_warn(dev, "Failed to attach to PM Domain : %d\n", ret);
+		return ret;
+	}
+
+	ret = auxdrv->probe(auxdev, auxiliary_match_id(auxdrv->id_table, auxdev));
+	if (ret)
+		dev_pm_domain_detach(dev, true);
+
+	return ret;
+}
+
+static int auxiliary_bus_remove(struct device *dev)
+{
+	struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+	int ret = 0;
+
+	if (auxdrv->remove)
+		ret = auxdrv->remove(auxdev);
+	dev_pm_domain_detach(dev, true);
+
+	return ret;
+}
+
+static void auxiliary_bus_shutdown(struct device *dev)
+{
+	struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+
+	if (auxdrv->shutdown)
+		auxdrv->shutdown(auxdev);
+}
+
+static struct bus_type auxiliary_bus_type = {
+	.name = "auxiliary",
+	.probe = auxiliary_bus_probe,
+	.remove = auxiliary_bus_remove,
+	.shutdown = auxiliary_bus_shutdown,
+	.match = auxiliary_match,
+	.uevent = auxiliary_uevent,
+	.pm = &auxiliary_dev_pm_ops,
+};
+
+/**
+ * auxiliary_device_init - check auxiliary_device and initialize
+ * @auxdev: auxiliary device struct
+ *
+ * This is the first step in the two-step process to register an auxiliary_device.
+ *
+ * When this function returns an error code, then the device_initialize will *not* have
+ * been performed, and the caller will be responsible to free any memory allocated for the
+ * auxiliary_device in the error path directly.
+ *
+ * It returns 0 on success.  On success, the device_initialize has been performed.  After this
+ * point any error unwinding will need to include a call to auxiliary_device_uninit().
+ * In this post-initialize error scenario, a call to the device's .release callback will be
+ * triggered, and all memory clean-up is expected to be handled there.
+ */
+int auxiliary_device_init(struct auxiliary_device *auxdev)
+{
+	struct device *dev = &auxdev->dev;
+
+	if (!dev->parent) {
+		pr_err("auxiliary_device has a NULL dev->parent\n");
+		return -EINVAL;
+	}
+
+	if (!auxdev->name) {
+		pr_err("auxiliary_device has a NULL name\n");
+		return -EINVAL;
+	}
+
+	dev->bus = &auxiliary_bus_type;
+	device_initialize(&auxdev->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(auxiliary_device_init);
+
+/**
+ * __auxiliary_device_add - add an auxiliary bus device
+ * @auxdev: auxiliary bus device to add to the bus
+ * @modname: name of the parent device's driver module
+ *
+ * This is the second step in the two-step process to register an auxiliary_device.
+ *
+ * This function must be called after a successful call to auxiliary_device_init(), which
+ * will perform the device_initialize.  This means that if this returns an error code, then a
+ * call to auxiliary_device_uninit() must be performed so that the .release callback will
+ * be triggered to free the memory associated with the auxiliary_device.
+ *
+ * The expectation is that users will call the "auxiliary_device_add" macro so that the caller's
+ * KBUILD_MODNAME is automatically inserted for the modname parameter.  Only if a user requires
+ * a custom name would this version be called directly.
+ */
+int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname)
+{
+	struct device *dev = &auxdev->dev;
+	int ret;
+
+	if (!modname) {
+		pr_err("auxiliary device modname is NULL\n");
+		return -EINVAL;
+	}
+
+	ret = dev_set_name(dev, "%s.%s.%d", modname, auxdev->name, auxdev->id);
+	if (ret) {
+		pr_err("auxiliary device dev_set_name failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = device_add(dev);
+	if (ret)
+		dev_err(dev, "adding auxiliary device failed!: %d\n", ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__auxiliary_device_add);
+
+/**
+ * auxiliary_find_device - auxiliary device iterator for locating a particular device.
+ * @start: Device to begin with
+ * @data: Data to pass to match function
+ * @match: Callback function to check device
+ *
+ * This function returns a reference to a device that is 'found'
+ * for later use, as determined by the @match callback.
+ *
+ * The callback should return 0 if the device doesn't match and non-zero
+ * if it does.  If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more devices.
+ */
+struct auxiliary_device *
+auxiliary_find_device(struct device *start, const void *data,
+		      int (*match)(struct device *dev, const void *data))
+{
+	struct device *dev;
+
+	dev = bus_find_device(&auxiliary_bus_type, start, data, match);
+	if (!dev)
+		return NULL;
+
+	return to_auxiliary_dev(dev);
+}
+EXPORT_SYMBOL_GPL(auxiliary_find_device);
+
+/**
+ * __auxiliary_driver_register - register a driver for auxiliary bus devices
+ * @auxdrv: auxiliary_driver structure
+ * @owner: owning module/driver
+ * @modname: KBUILD_MODNAME for parent driver
+ */
+int __auxiliary_driver_register(struct auxiliary_driver *auxdrv, struct module *owner,
+				const char *modname)
+{
+	if (WARN_ON(!auxdrv->probe) || WARN_ON(!auxdrv->id_table))
+		return -EINVAL;
+
+	if (auxdrv->name)
+		auxdrv->driver.name = kasprintf(GFP_KERNEL, "%s.%s", modname, auxdrv->name);
+	else
+		auxdrv->driver.name = kasprintf(GFP_KERNEL, "%s", modname);
+	if (!auxdrv->driver.name)
+		return -ENOMEM;
+
+	auxdrv->driver.owner = owner;
+	auxdrv->driver.bus = &auxiliary_bus_type;
+	auxdrv->driver.mod_name = modname;
+
+	return driver_register(&auxdrv->driver);
+}
+EXPORT_SYMBOL_GPL(__auxiliary_driver_register);
+
+/**
+ * auxiliary_driver_unregister - unregister a driver
+ * @auxdrv: auxiliary_driver structure
+ */
+void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
+{
+	driver_unregister(&auxdrv->driver);
+	kfree(auxdrv->driver.name);
+}
+EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
+
+static int __init auxiliary_bus_init(void)
+{
+	return bus_register(&auxiliary_bus_type);
+}
+
+static void __exit auxiliary_bus_exit(void)
+{
+	bus_unregister(&auxiliary_bus_type);
+}
+
+module_init(auxiliary_bus_init);
+module_exit(auxiliary_bus_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Auxiliary Bus");
+MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>");
+MODULE_AUTHOR("Kiran Patil <kiran.patil@intel.com>");
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
new file mode 100644
index 000000000000..282fbf7bf9af
--- /dev/null
+++ b/include/linux/auxiliary_bus.h
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2019-2020 Intel Corporation
+ *
+ * Please see Documentation/driver-api/auxiliary_bus.rst for more information.
+ */
+
+#ifndef _AUXILIARY_BUS_H_
+#define _AUXILIARY_BUS_H_
+
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
+
+struct auxiliary_device {
+	struct device dev;
+	const char *name;
+	u32 id;
+};
+
+struct auxiliary_driver {
+	int (*probe)(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id);
+	int (*remove)(struct auxiliary_device *auxdev);
+	void (*shutdown)(struct auxiliary_device *auxdev);
+	int (*suspend)(struct auxiliary_device *auxdev, pm_message_t state);
+	int (*resume)(struct auxiliary_device *auxdev);
+	const char *name;
+	struct device_driver driver;
+	const struct auxiliary_device_id *id_table;
+};
+
+static inline struct auxiliary_device *to_auxiliary_dev(struct device *dev)
+{
+	return container_of(dev, struct auxiliary_device, dev);
+}
+
+static inline struct auxiliary_driver *to_auxiliary_drv(struct device_driver *drv)
+{
+	return container_of(drv, struct auxiliary_driver, driver);
+}
+
+int auxiliary_device_init(struct auxiliary_device *auxdev);
+int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname);
+#define auxiliary_device_add(auxdev) __auxiliary_device_add(auxdev, KBUILD_MODNAME)
+
+static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
+{
+	put_device(&auxdev->dev);
+}
+
+static inline void auxiliary_device_delete(struct auxiliary_device *auxdev)
+{
+	device_del(&auxdev->dev);
+}
+
+int __auxiliary_driver_register(struct auxiliary_driver *auxdrv, struct module *owner,
+				const char *modname);
+#define auxiliary_driver_register(auxdrv) \
+	__auxiliary_driver_register(auxdrv, THIS_MODULE, KBUILD_MODNAME)
+
+void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv);
+
+/**
+ * module_auxiliary_driver() - Helper macro for registering an auxiliary driver
+ * @__auxiliary_driver: auxiliary driver struct
+ *
+ * Helper macro for auxiliary drivers which do not do anything special in
+ * module init/exit. This eliminates a lot of boilerplate. Each module may only
+ * use this macro once, and calling it replaces module_init() and module_exit()
+ */
+#define module_auxiliary_driver(__auxiliary_driver) \
+	module_driver(__auxiliary_driver, auxiliary_driver_register, auxiliary_driver_unregister)
+
+struct auxiliary_device *
+auxiliary_find_device(struct device *start, const void *data,
+		      int (*match)(struct device *dev, const void *data));
+
+#endif /* _AUXILIARY_BUS_H_ */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 5b08a473cdba..c425290b21e2 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -838,4 +838,12 @@ struct mhi_device_id {
 	kernel_ulong_t driver_data;
 };
 
+#define AUXILIARY_NAME_SIZE 32
+#define AUXILIARY_MODULE_PREFIX "auxiliary:"
+
+struct auxiliary_device_id {
+	char name[AUXILIARY_NAME_SIZE];
+	kernel_ulong_t driver_data;
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 27007c18e754..e377f52dbfa3 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -243,5 +243,8 @@ int main(void)
 	DEVID(mhi_device_id);
 	DEVID_FIELD(mhi_device_id, chan);
 
+	DEVID(auxiliary_device_id);
+	DEVID_FIELD(auxiliary_device_id, name);
+
 	return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 2417dd1dee33..fb4827027536 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1364,6 +1364,13 @@ static int do_mhi_entry(const char *filename, void *symval, char *alias)
 {
 	DEF_FIELD_ADDR(symval, mhi_device_id, chan);
 	sprintf(alias, MHI_DEVICE_MODALIAS_FMT, *chan);
+	return 1;
+}
+
+static int do_auxiliary_entry(const char *filename, void *symval, char *alias)
+{
+	DEF_FIELD_ADDR(symval, auxiliary_device_id, name);
+	sprintf(alias, AUXILIARY_MODULE_PREFIX "%s", *name);
 
 	return 1;
 }
@@ -1442,6 +1449,7 @@ static const struct devtable devtable[] = {
 	{"tee", SIZE_tee_client_device_id, do_tee_entry},
 	{"wmi", SIZE_wmi_device_id, do_wmi_entry},
 	{"mhi", SIZE_mhi_device_id, do_mhi_entry},
+	{"auxiliary", SIZE_auxiliary_device_id, do_auxiliary_entry},
 };
 
 /* Create MODULE_ALIAS() statements.
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 02/10] ASoC: SOF: Introduce descriptors for SOF client
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 01/10] Add auxiliary bus support Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 03/10] ASoC: SOF: Create client driver for IPC test Dave Ertman
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

A client in the SOF (Sound Open Firmware) context is a
device that needs to communicate with the DSP via IPC
messages. The SOF core is responsible for serializing the
IPC messages to the DSP from the different clients. One
example of an SOF client would be an IPC test client that
floods the DSP with test IPC messages to validate if the
serialization works as expected. Multi-client support will
also add the ability to split the existing audio cards
into multiple ones, so as to e.g. to deal with HDMI with a
dedicated client instead of adding HDMI to all cards.

This patch introduces descriptors for SOF client driver
and SOF client device along with APIs for registering
and unregistering a SOF client driver, sending IPCs from
a client device and accessing the SOF core debugfs root entry.

Along with this, add a couple of new members to struct
snd_sof_dev that will be used for maintaining the list of
clients.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/Kconfig      |  19 ++++++
 sound/soc/sof/Makefile     |   3 +
 sound/soc/sof/core.c       |   2 +
 sound/soc/sof/sof-client.c | 115 +++++++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-client.h |  66 +++++++++++++++++++++
 sound/soc/sof/sof-priv.h   |   9 +++
 6 files changed, 214 insertions(+)
 create mode 100644 sound/soc/sof/sof-client.c
 create mode 100644 sound/soc/sof/sof-client.h

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index 8c1f0829de40..31e9911098fc 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -50,6 +50,24 @@ config SND_SOC_SOF_DEBUG_PROBES
 	  Say Y if you want to enable probes.
 	  If unsure, select "N".
 
+config SND_SOC_SOF_CLIENT
+	tristate
+	select AUXILIARY_BUS
+	help
+	  This option is not user-selectable but automagically handled by
+	  'select' statements at a higher level.
+
+config SND_SOC_SOF_CLIENT_SUPPORT
+	bool "SOF enable clients"
+	depends on SND_SOC_SOF
+	help
+	  This adds support for auxiliary client devices to separate out the debug
+	  functionality for IPC tests, probes etc. into separate devices. This
+	  option would also allow adding client devices based on DSP firmware
+	  capabilities and ACPI/OF device information.
+	  Say Y if you want to enable clients with SOF.
+	  If unsure select "N".
+
 config SND_SOC_SOF_DEVELOPER_SUPPORT
 	bool "SOF developer options support"
 	depends on EXPERT
@@ -186,6 +204,7 @@ endif ## SND_SOC_SOF_DEVELOPER_SUPPORT
 
 config SND_SOC_SOF
 	tristate
+	select SND_SOC_SOF_CLIENT if SND_SOC_SOF_CLIENT_SUPPORT
 	select SND_SOC_TOPOLOGY
 	select SND_SOC_SOF_NOCODEC if SND_SOC_SOF_NOCODEC_SUPPORT
 	help
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index 05718dfe6cd2..5e46f25a3851 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -2,6 +2,7 @@
 
 snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
 		control.o trace.o utils.o sof-audio.o
+snd-sof-client-objs := sof-client.o
 snd-sof-$(CONFIG_SND_SOC_SOF_DEBUG_PROBES) += probe.o compress.o
 
 snd-sof-pci-objs := sof-pci-dev.o
@@ -18,6 +19,8 @@ obj-$(CONFIG_SND_SOC_SOF_ACPI) += snd-sof-acpi.o
 obj-$(CONFIG_SND_SOC_SOF_OF) += snd-sof-of.o
 obj-$(CONFIG_SND_SOC_SOF_PCI) += snd-sof-pci.o
 
+obj-$(CONFIG_SND_SOC_SOF_CLIENT) += snd-sof-client.o
+
 obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/
 obj-$(CONFIG_SND_SOC_SOF_IMX_TOPLEVEL) += imx/
 obj-$(CONFIG_SND_SOC_SOF_XTENSA) += xtensa/
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index adc7c37145d6..72a97219395f 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -314,8 +314,10 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
 	INIT_LIST_HEAD(&sdev->widget_list);
 	INIT_LIST_HEAD(&sdev->dai_list);
 	INIT_LIST_HEAD(&sdev->route_list);
+	INIT_LIST_HEAD(&sdev->client_list);
 	spin_lock_init(&sdev->ipc_lock);
 	spin_lock_init(&sdev->hw_lock);
+	mutex_init(&sdev->client_mutex);
 
 	if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE))
 		INIT_WORK(&sdev->probe_work, sof_probe_work);
diff --git a/sound/soc/sof/sof-client.c b/sound/soc/sof/sof-client.c
new file mode 100644
index 000000000000..dd75a0ba4c28
--- /dev/null
+++ b/sound/soc/sof/sof-client.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2020 Intel Corporation. All rights reserved.
+//
+// Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
+//
+
+#include <linux/debugfs.h>
+#include <linux/errno.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include "sof-client.h"
+#include "sof-priv.h"
+
+static void sof_client_auxdev_release(struct device *dev)
+{
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+	struct sof_client_dev *cdev = auxiliary_dev_to_sof_client_dev(auxdev);
+
+	kfree(cdev);
+}
+
+static struct sof_client_dev *sof_client_dev_alloc(struct snd_sof_dev *sdev, const char *name,
+						   u32 id)
+{
+	struct sof_client_dev *cdev;
+	struct auxiliary_device *auxdev;
+	int ret;
+
+	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
+	if (!cdev)
+		return ERR_PTR(-ENOMEM);
+
+	cdev->sdev = sdev;
+	auxdev = &cdev->auxdev;
+	auxdev->name = name;
+	auxdev->dev.parent = sdev->dev;
+	auxdev->dev.release = sof_client_auxdev_release;
+	auxdev->id = id;
+
+	ret = auxiliary_device_init(auxdev);
+	if (ret < 0) {
+		dev_err(sdev->dev, "error: failed to initialize client dev %s\n", name);
+		goto err_free;
+	}
+
+	return cdev;
+
+err_free:
+	kfree(cdev);
+	return NULL;
+}
+
+int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name, u32 id)
+{
+	struct sof_client_dev *cdev;
+	int ret;
+
+	cdev = sof_client_dev_alloc(sdev, name, id);
+	if (IS_ERR_OR_NULL(cdev))
+		return PTR_ERR(cdev);
+
+	ret = auxiliary_device_add(&cdev->auxdev);
+	if (ret < 0) {
+		dev_err(sdev->dev, "error: failed to add client dev %s\n", name);
+		/* cdev will be freed when the release callback is invoked through put_device() */
+		auxiliary_device_uninit(&cdev->auxdev);
+		return ret;
+	}
+
+	/* add to list of SOF client devices */
+	mutex_lock(&sdev->client_mutex);
+	list_add(&cdev->list, &sdev->client_list);
+	mutex_unlock(&sdev->client_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_dev_register, SND_SOC_SOF_CLIENT);
+
+void sof_client_dev_unregister(struct snd_sof_dev *sdev, const char *name, u32 id)
+{
+	struct sof_client_dev *cdev, *_cdev;
+
+	mutex_lock(&sdev->client_mutex);
+
+	/* cdev will be freed when the release callback for the auxiliary device is invoked */
+	list_for_each_entry_safe(cdev, _cdev, &sdev->client_list, list) {
+		if (!strcmp(cdev->auxdev.name, name) && cdev->auxdev.id == id) {
+			auxiliary_device_delete(&cdev->auxdev);
+			auxiliary_device_uninit(&cdev->auxdev);
+			break;
+		}
+	}
+
+	mutex_unlock(&sdev->client_mutex);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_dev_unregister, SND_SOC_SOF_CLIENT);
+
+int sof_client_ipc_tx_message(struct sof_client_dev *cdev, u32 header, void *msg_data,
+			      size_t msg_bytes, void *reply_data, size_t reply_bytes)
+{
+	return sof_ipc_tx_message(cdev->sdev->ipc, header, msg_data, msg_bytes,
+				  reply_data, reply_bytes);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_ipc_tx_message, SND_SOC_SOF_CLIENT);
+
+struct dentry *sof_client_get_debugfs_root(struct sof_client_dev *cdev)
+{
+	return cdev->sdev->debugfs_root;
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_get_debugfs_root, SND_SOC_SOF_CLIENT);
+
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/sof/sof-client.h b/sound/soc/sof/sof-client.h
new file mode 100644
index 000000000000..429282df9f65
--- /dev/null
+++ b/sound/soc/sof/sof-client.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOUND_SOC_SOF_CLIENT_H
+#define __SOUND_SOC_SOF_CLIENT_H
+
+#include <linux/auxiliary_bus.h>
+#include <linux/device.h>
+#include <linux/idr.h>
+#include <linux/list.h>
+
+#define SOF_CLIENT_PROBE_TIMEOUT_MS 2000
+
+struct snd_sof_dev;
+
+/* SOF client device */
+struct sof_client_dev {
+	struct auxiliary_device auxdev;
+	struct snd_sof_dev *sdev;
+	struct list_head list;	/* item in SOF core client dev list */
+	void *data;
+};
+
+/* client-specific ops, all optional */
+struct sof_client_ops {
+	int (*client_ipc_rx)(struct sof_client_dev *cdev, u32 msg_cmd);
+};
+
+struct sof_client_drv {
+	const struct sof_client_ops ops;
+	struct auxiliary_driver auxiliary_drv;
+};
+
+#define auxiliary_dev_to_sof_client_dev(auxiliary_dev) \
+	container_of(auxiliary_dev, struct sof_client_dev, auxdev)
+
+static inline int sof_client_drv_register(struct sof_client_drv *drv)
+{
+	return auxiliary_driver_register(&drv->auxiliary_drv);
+}
+
+static inline void sof_client_drv_unregister(struct sof_client_drv *drv)
+{
+	auxiliary_driver_unregister(&drv->auxiliary_drv);
+}
+
+int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name, u32 id);
+void sof_client_dev_unregister(struct snd_sof_dev *sdev, const char *name, u32 id);
+
+int sof_client_ipc_tx_message(struct sof_client_dev *cdev, u32 header, void *msg_data,
+			      size_t msg_bytes, void *reply_data, size_t reply_bytes);
+
+struct dentry *sof_client_get_debugfs_root(struct sof_client_dev *cdev);
+
+/**
+ * module_sof_client_driver() - Helper macro for registering an SOF Client
+ * driver
+ * @__sof_client_driver: SOF client driver struct
+ *
+ * Helper macro for SOF client drivers which do not do anything special in
+ * module init/exit. This eliminates a lot of boilerplate. Each module may only
+ * use this macro once, and calling it replaces module_init() and module_exit()
+ */
+#define module_sof_client_driver(__sof_client_driver) \
+	module_driver(__sof_client_driver, sof_client_drv_register, sof_client_drv_unregister)
+
+#endif
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 0aed2a7ab858..dceac73b858f 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -442,6 +442,15 @@ struct snd_sof_dev {
 
 	bool msi_enabled;
 
+	/*
+	 * Used to keep track of registered client devices so that they can be removed when the
+	 * parent SOF module is removed.
+	 */
+	struct list_head client_list;
+
+	/* mutex to protect client list */
+	struct mutex client_mutex;
+
 	void *private;			/* core does not touch this */
 };
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 03/10] ASoC: SOF: Create client driver for IPC test
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 01/10] Add auxiliary bus support Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 02/10] ASoC: SOF: Introduce descriptors for SOF client Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 04/10] ASoC: SOF: ops: Add ops for client registration Dave Ertman
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Create an SOF client driver for IPC flood test. This
driver is used to set up the debugfs entries and the
read/write ops for initiating the IPC flood test that
would be used to measure the min/max/avg response times
for sending IPCs to the DSP. The debugfs ops definitions
in the driver is existing code that has been copied
from the core. These will be removed from the SOF core
making is less monolithic and easier to maintain.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/Kconfig               |  10 +
 sound/soc/sof/Makefile              |   4 +
 sound/soc/sof/sof-ipc-test-client.c | 321 ++++++++++++++++++++++++++++
 3 files changed, 335 insertions(+)
 create mode 100644 sound/soc/sof/sof-ipc-test-client.c

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index 31e9911098fc..13bde36cc5d7 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -190,6 +190,16 @@ config SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST
 	  Say Y if you want to enable IPC flood test.
 	  If unsure, select "N".
 
+config SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT
+	tristate "SOF enable IPC flood test client"
+	depends on SND_SOC_SOF_CLIENT
+	help
+	  This option enables a separate client device for IPC flood test
+	  which can be used to flood the DSP with test IPCs and gather stats
+	  about response times.
+	  Say Y if you want to enable IPC flood test.
+	  If unsure, select "N".
+
 config SND_SOC_SOF_DEBUG_RETAIN_DSP_CONTEXT
 	bool "SOF retain DSP context on any FW exceptions"
 	help
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index 5e46f25a3851..baa93fe2cc9a 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -9,6 +9,8 @@ snd-sof-pci-objs := sof-pci-dev.o
 snd-sof-acpi-objs := sof-acpi-dev.o
 snd-sof-of-objs := sof-of-dev.o
 
+snd-sof-ipc-test-objs := sof-ipc-test-client.o
+
 snd-sof-nocodec-objs := nocodec.o
 
 obj-$(CONFIG_SND_SOC_SOF) += snd-sof.o
@@ -21,6 +23,8 @@ obj-$(CONFIG_SND_SOC_SOF_PCI) += snd-sof-pci.o
 
 obj-$(CONFIG_SND_SOC_SOF_CLIENT) += snd-sof-client.o
 
+obj-$(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT) += snd-sof-ipc-test.o
+
 obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/
 obj-$(CONFIG_SND_SOC_SOF_IMX_TOPLEVEL) += imx/
 obj-$(CONFIG_SND_SOC_SOF_XTENSA) += xtensa/
diff --git a/sound/soc/sof/sof-ipc-test-client.c b/sound/soc/sof/sof-ipc-test-client.c
new file mode 100644
index 000000000000..b4d803b9139b
--- /dev/null
+++ b/sound/soc/sof/sof-ipc-test-client.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2020 Intel Corporation. All rights reserved.
+//
+// Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
+//
+
+#include <linux/auxiliary_bus.h>
+#include <linux/completion.h>
+#include <linux/debugfs.h>
+#include <linux/ktime.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <sound/sof/header.h>
+#include "sof-client.h"
+
+#define MAX_IPC_FLOOD_DURATION_MS 1000
+#define MAX_IPC_FLOOD_COUNT 10000
+#define IPC_FLOOD_TEST_RESULT_LEN 512
+#define SOF_IPC_CLIENT_SUSPEND_DELAY_MS 3000
+
+struct sof_ipc_client_data {
+	struct dentry *dfs_root;
+	char *buf;
+};
+
+/*
+ * helper function to perform the flood test. Only one of the two params, ipc_duration_ms
+ * or ipc_count, will be non-zero and will determine the type of test
+ */
+static int sof_debug_ipc_flood_test(struct sof_client_dev *cdev, unsigned long ipc_duration_ms,
+				    unsigned long ipc_count)
+{
+	struct sof_ipc_client_data *ipc_client_data = cdev->data;
+	struct device *dev = &cdev->auxdev.dev;
+	struct sof_ipc_cmd_hdr hdr;
+	struct sof_ipc_reply reply;
+	u64 min_response_time = U64_MAX;
+	u64 avg_response_time = 0;
+	u64 max_response_time = 0;
+	ktime_t cur;
+	ktime_t test_end;
+	int i = 0;
+	int ret = 0;
+	bool end_test = false;
+
+	/* configure test IPC */
+	hdr.cmd = SOF_IPC_GLB_TEST_MSG | SOF_IPC_TEST_IPC_FLOOD;
+	hdr.size = sizeof(hdr);
+
+	/* set test end time for duration flood test */
+	test_end = ktime_get_ns() + ipc_duration_ms * NSEC_PER_MSEC;
+
+	/* send test IPC's */
+	do {
+		ktime_t start;
+		u64 ipc_response_time;
+
+		start = ktime_get();
+		ret = sof_client_ipc_tx_message(cdev, hdr.cmd, &hdr, hdr.size, &reply,
+						sizeof(reply));
+		if (ret < 0)
+			break;
+		cur = ktime_get();
+
+		i++;
+
+		/* compute min and max response times */
+		ipc_response_time = ktime_to_ns(ktime_sub(cur, start));
+		min_response_time = min(min_response_time, ipc_response_time);
+		max_response_time = max(max_response_time, ipc_response_time);
+
+		/* sum up response times */
+		avg_response_time += ipc_response_time;
+
+		/* end test? */
+		if (ipc_count && i == ipc_count)
+			end_test = true;
+		else if (ipc_duration_ms && (ktime_to_ns(cur) >= test_end))
+			end_test = true;
+
+	} while (!end_test);
+
+	if (ret < 0)
+		return ret;
+
+	/* return if the first IPC fails */
+	if (!i)
+		return ret;
+
+	/* compute average response time */
+	DIV_ROUND_CLOSEST(avg_response_time, i);
+
+	/* clear previous test output */
+	memset(ipc_client_data->buf, 0, IPC_FLOOD_TEST_RESULT_LEN);
+
+	if (!ipc_count) {
+		dev_dbg(dev, "IPC Flood test duration: %lums\n", ipc_duration_ms);
+		snprintf(ipc_client_data->buf, IPC_FLOOD_TEST_RESULT_LEN,
+			 "IPC Flood test duration: %lums\n", ipc_duration_ms);
+	}
+
+	dev_dbg(dev,
+		"IPC Flood count: %d, Avg response time: %lluns\n", i, avg_response_time);
+	dev_dbg(dev, "Max response time: %lluns\n", max_response_time);
+	dev_dbg(dev, "Min response time: %lluns\n", min_response_time);
+
+	/* format output string and save test results */
+	snprintf(ipc_client_data->buf + strlen(ipc_client_data->buf),
+		 IPC_FLOOD_TEST_RESULT_LEN - strlen(ipc_client_data->buf),
+		 "IPC Flood count: %d\nAvg response time: %lluns\n", i, avg_response_time);
+
+	snprintf(ipc_client_data->buf + strlen(ipc_client_data->buf),
+		 IPC_FLOOD_TEST_RESULT_LEN - strlen(ipc_client_data->buf),
+		 "Max response time: %lluns\nMin response time: %lluns\n",
+		 max_response_time, min_response_time);
+
+	return ret;
+}
+
+/*
+ * Writing to the debugfs entry initiates the IPC flood test based on
+ * the IPC count or the duration specified by the user.
+ */
+static ssize_t sof_ipc_dfsentry_write(struct file *file, const char __user *buffer,
+				      size_t count, loff_t *ppos)
+{
+	struct dentry *dentry = file->f_path.dentry;
+	struct sof_client_dev *cdev = file->private_data;
+	struct device *dev = &cdev->auxdev.dev;
+	unsigned long ipc_duration_ms = 0;
+	bool flood_duration_test;
+	unsigned long ipc_count = 0;
+	char *string;
+	size_t size;
+	int err;
+	int ret;
+
+	string = kzalloc(count, GFP_KERNEL);
+	if (!string)
+		return -ENOMEM;
+
+	size = simple_write_to_buffer(string, count, ppos, buffer, count);
+
+	flood_duration_test = !strcmp(dentry->d_name.name, "ipc_flood_duration_ms");
+
+	/* limit max duration/ipc count for flood test */
+	if (flood_duration_test) {
+		ret = kstrtoul(string, 0, &ipc_duration_ms);
+		if (ret < 0)
+			goto out;
+
+		if (!ipc_duration_ms) {
+			ret = size;
+			goto out;
+		}
+
+		ipc_duration_ms = min_t(unsigned long, ipc_duration_ms, MAX_IPC_FLOOD_DURATION_MS);
+	} else {
+		ret = kstrtoul(string, 0, &ipc_count);
+		if (ret < 0)
+			goto out;
+
+		if (!ipc_count) {
+			ret = size;
+			goto out;
+		}
+
+		ipc_count = min_t(unsigned long, ipc_count, MAX_IPC_FLOOD_COUNT);
+	}
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(dev, "error: debugfs write failed to resume %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto out;
+	}
+
+	ret = sof_debug_ipc_flood_test(cdev, ipc_duration_ms, ipc_count);
+
+	pm_runtime_mark_last_busy(dev);
+	err = pm_runtime_put_autosuspend(dev);
+	if (err < 0) {
+		ret = err;
+		goto out;
+	}
+
+	/* return size if test is successful */
+	if (ret >= 0)
+		ret = size;
+out:
+	kfree(string);
+	return ret;
+}
+
+/* return the result of the last IPC flood test */
+static ssize_t sof_ipc_dfsentry_read(struct file *file, char __user *buffer,
+				     size_t count, loff_t *ppos)
+{
+	struct sof_client_dev *cdev = file->private_data;
+	struct sof_ipc_client_data *ipc_client_data = cdev->data;
+	size_t size_ret;
+
+	if (*ppos)
+		return 0;
+
+	/* return results of the last IPC test */
+	count = min_t(size_t, count, strlen(ipc_client_data->buf));
+	size_ret = copy_to_user(buffer, ipc_client_data->buf, count);
+	if (size_ret)
+		return -EFAULT;
+
+	*ppos += count;
+	return count;
+}
+
+static const struct file_operations sof_ipc_dfs_fops = {
+	.open = simple_open,
+	.read = sof_ipc_dfsentry_read,
+	.llseek = default_llseek,
+	.write = sof_ipc_dfsentry_write,
+};
+
+/*
+ * The IPC test client creates a couple of debugfs entries that will be used
+ * flood tests. Users can write to these entries to execute the IPC flood test
+ * by specifying either the number of IPCs to flood the DSP with or the duration
+ * (in ms) for which the DSP should be flooded with test IPCs. At the
+ * end of each test, the average, min and max response times are reported back.
+ * The results of the last flood test can be accessed by reading the debugfs
+ * entries.
+ */
+static int sof_ipc_test_probe(struct auxiliary_device *auxdev,
+			      const struct auxiliary_device_id *id)
+{
+	struct sof_client_dev *cdev = auxiliary_dev_to_sof_client_dev(auxdev);
+	struct sof_ipc_client_data *ipc_client_data;
+
+	/* allocate memory for client data */
+	ipc_client_data = devm_kzalloc(&auxdev->dev, sizeof(*ipc_client_data), GFP_KERNEL);
+	if (!ipc_client_data)
+		return -ENOMEM;
+
+	ipc_client_data->buf = devm_kzalloc(&auxdev->dev, IPC_FLOOD_TEST_RESULT_LEN, GFP_KERNEL);
+	if (!ipc_client_data->buf)
+		return -ENOMEM;
+
+	cdev->data = ipc_client_data;
+
+	/* create debugfs root folder with device name under parent SOF dir */
+	ipc_client_data->dfs_root = debugfs_create_dir(dev_name(&auxdev->dev),
+						       sof_client_get_debugfs_root(cdev));
+
+	/* create read-write ipc_flood_count debugfs entry */
+	debugfs_create_file("ipc_flood_count", 0644, ipc_client_data->dfs_root,
+			    cdev, &sof_ipc_dfs_fops);
+
+	/* create read-write ipc_flood_duration_ms debugfs entry */
+	debugfs_create_file("ipc_flood_duration_ms", 0644, ipc_client_data->dfs_root,
+			    cdev, &sof_ipc_dfs_fops);
+
+	/* enable runtime PM */
+	pm_runtime_set_autosuspend_delay(&auxdev->dev, SOF_IPC_CLIENT_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(&auxdev->dev);
+	pm_runtime_enable(&auxdev->dev);
+	pm_runtime_mark_last_busy(&auxdev->dev);
+	pm_runtime_idle(&auxdev->dev);
+
+	return 0;
+}
+
+static int sof_ipc_test_cleanup(struct auxiliary_device *auxdev)
+{
+	struct sof_client_dev *cdev = auxiliary_dev_to_sof_client_dev(auxdev);
+	struct sof_ipc_client_data *ipc_client_data = cdev->data;
+
+	pm_runtime_disable(&auxdev->dev);
+
+	debugfs_remove_recursive(ipc_client_data->dfs_root);
+
+	return 0;
+}
+
+static int sof_ipc_test_remove(struct auxiliary_device *auxdev)
+{
+	return sof_ipc_test_cleanup(auxdev);
+}
+
+static void sof_ipc_test_shutdown(struct auxiliary_device *auxdev)
+{
+	sof_ipc_test_cleanup(auxdev);
+}
+
+static const struct auxiliary_device_id sof_ipc_auxbus_id_table[] = {
+	{ .name = "snd_sof_client.ipc_test" },
+	{},
+};
+MODULE_DEVICE_TABLE(auxiliary, sof_ipc_auxbus_id_table);
+
+/*
+ * No need for driver pm_ops as the generic pm callbacks in the auxiliary bus type are enough to
+ * ensure that the parent SOF device resumes to bring the DSP back to D0.
+ * driver name will be set based on KBUILD_MODNAME.
+ */
+static struct sof_client_drv sof_ipc_test_client_drv = {
+	.auxiliary_drv = {
+		.id_table = sof_ipc_auxbus_id_table,
+		.probe = sof_ipc_test_probe,
+		.remove = sof_ipc_test_remove,
+		.shutdown = sof_ipc_test_shutdown,
+	},
+};
+
+module_sof_client_driver(sof_ipc_test_client_drv);
+
+MODULE_DESCRIPTION("SOF IPC Test Client Driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SND_SOC_SOF_CLIENT);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 04/10] ASoC: SOF: ops: Add ops for client registration
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (2 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 03/10] ASoC: SOF: Create client driver for IPC test Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 05/10] ASoC: SOF: Intel: Define " Dave Ertman
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add new ops for registering/unregistering clients based
on DSP capabilities and/or DT information.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/core.c     | 10 ++++++++++
 sound/soc/sof/ops.h      | 14 ++++++++++++++
 sound/soc/sof/sof-priv.h |  4 ++++
 3 files changed, 28 insertions(+)

diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index 72a97219395f..ddb9a12d5aac 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -246,8 +246,17 @@ static int sof_probe_continue(struct snd_sof_dev *sdev)
 	if (plat_data->sof_probe_complete)
 		plat_data->sof_probe_complete(sdev->dev);
 
+	/* If registering certain clients fails, unregister the previously registered clients. */
+	ret = snd_sof_register_clients(sdev);
+	if (ret < 0) {
+		dev_err(sdev->dev, "error: failed to register clients %d\n", ret);
+		goto client_reg_err;
+	}
+
 	return 0;
 
+client_reg_err:
+	snd_sof_unregister_clients(sdev);
 fw_trace_err:
 	snd_sof_free_trace(sdev);
 fw_run_err:
@@ -356,6 +365,7 @@ int snd_sof_device_remove(struct device *dev)
 			dev_warn(dev, "error: %d failed to prepare DSP for device removal",
 				 ret);
 
+		snd_sof_unregister_clients(sdev);
 		snd_sof_fw_unload(sdev);
 		snd_sof_ipc_free(sdev);
 		snd_sof_free_debug(sdev);
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index b21632f5511a..00370f8bcd75 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -470,6 +470,20 @@ snd_sof_set_mach_params(const struct snd_soc_acpi_mach *mach,
 		sof_ops(sdev)->set_mach_params(mach, dev);
 }
 
+static inline int snd_sof_register_clients(struct snd_sof_dev *sdev)
+{
+	if (sof_ops(sdev) && sof_ops(sdev)->register_clients)
+		return sof_ops(sdev)->register_clients(sdev);
+
+	return 0;
+}
+
+static inline void snd_sof_unregister_clients(struct snd_sof_dev *sdev)
+{
+	if (sof_ops(sdev) && sof_ops(sdev)->unregister_clients)
+		sof_ops(sdev)->unregister_clients(sdev);
+}
+
 static inline const struct snd_sof_dsp_ops
 *sof_get_ops(const struct sof_dev_desc *d,
 	     const struct sof_ops_table mach_ops[], int asize)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index dceac73b858f..cca239c09d0e 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -252,6 +252,10 @@ struct snd_sof_dsp_ops {
 	void (*set_mach_params)(const struct snd_soc_acpi_mach *mach,
 				struct device *dev); /* optional */
 
+	/* client ops */
+	int (*register_clients)(struct snd_sof_dev *sdev); /* optional */
+	void (*unregister_clients)(struct snd_sof_dev *sdev); /* optional */
+
 	/* DAI ops */
 	struct snd_soc_dai_driver *drv;
 	int num_drv;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 05/10] ASoC: SOF: Intel: Define ops for client registration
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (3 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 04/10] ASoC: SOF: ops: Add ops for client registration Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 06/10] ASoC: SOF: Intel: Remove IPC flood test support in SOF core Dave Ertman
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Define client ops for Intel platforms. For now, we only add
2 IPC test clients that will be used for run tandem IPC flood
tests for.

For ACPI platforms, change the Kconfig to select
SND_SOC_SOF_PROBE_WORK_QUEUE to allow the ancillary driver
to probe when the client is registered.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/intel/Kconfig        |  9 +++++++
 sound/soc/sof/intel/Makefile       |  3 +++
 sound/soc/sof/intel/apl.c          | 16 ++++++++++++
 sound/soc/sof/intel/bdw.c          | 16 ++++++++++++
 sound/soc/sof/intel/byt.c          | 20 +++++++++++++++
 sound/soc/sof/intel/cnl.c          | 16 ++++++++++++
 sound/soc/sof/intel/intel-client.c | 40 ++++++++++++++++++++++++++++++
 sound/soc/sof/intel/intel-client.h | 26 +++++++++++++++++++
 8 files changed, 146 insertions(+)
 create mode 100644 sound/soc/sof/intel/intel-client.c
 create mode 100644 sound/soc/sof/intel/intel-client.h

diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig
index a066e08860cb..b449fa2f8005 100644
--- a/sound/soc/sof/intel/Kconfig
+++ b/sound/soc/sof/intel/Kconfig
@@ -13,6 +13,8 @@ config SND_SOC_SOF_INTEL_ACPI
 	def_tristate SND_SOC_SOF_ACPI
 	select SND_SOC_SOF_BAYTRAIL  if SND_SOC_SOF_BAYTRAIL_SUPPORT
 	select SND_SOC_SOF_BROADWELL if SND_SOC_SOF_BROADWELL_SUPPORT
+	select SND_SOC_SOF_PROBE_WORK_QUEUE if SND_SOC_SOF_CLIENT
+	select SND_SOC_SOF_INTEL_CLIENT if SND_SOC_SOF_CLIENT
 	help
 	  This option is not user-selectable but automagically handled by
 	  'select' statements at a higher level
@@ -29,6 +31,7 @@ config SND_SOC_SOF_INTEL_PCI
 	select SND_SOC_SOF_TIGERLAKE   if SND_SOC_SOF_TIGERLAKE_SUPPORT
 	select SND_SOC_SOF_ELKHARTLAKE if SND_SOC_SOF_ELKHARTLAKE_SUPPORT
 	select SND_SOC_SOF_JASPERLAKE  if SND_SOC_SOF_JASPERLAKE_SUPPORT
+	select SND_SOC_SOF_INTEL_CLIENT if SND_SOC_SOF_CLIENT
 	help
 	  This option is not user-selectable but automagically handled by
 	  'select' statements at a higher level
@@ -57,6 +60,12 @@ config SND_SOC_SOF_INTEL_COMMON
 	  This option is not user-selectable but automagically handled by
 	  'select' statements at a higher level
 
+config SND_SOC_SOF_INTEL_CLIENT
+	tristate
+	help
+	  This option is not user-selectable but automagically handled by
+	  'select' statements at a higher level.
+
 if SND_SOC_SOF_INTEL_ACPI
 
 config SND_SOC_SOF_BAYTRAIL_SUPPORT
diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile
index 72d85b25df7d..683e64c627c1 100644
--- a/sound/soc/sof/intel/Makefile
+++ b/sound/soc/sof/intel/Makefile
@@ -5,6 +5,8 @@ snd-sof-intel-bdw-objs := bdw.o
 
 snd-sof-intel-ipc-objs := intel-ipc.o
 
+snd-sof-intel-client-objs := intel-client.o
+
 snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \
 				 hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \
 				 hda-dai.o hda-bus.o \
@@ -18,3 +20,4 @@ obj-$(CONFIG_SND_SOC_SOF_BROADWELL) += snd-sof-intel-bdw.o
 obj-$(CONFIG_SND_SOC_SOF_INTEL_HIFI_EP_IPC) += snd-sof-intel-ipc.o
 obj-$(CONFIG_SND_SOC_SOF_HDA_COMMON) += snd-sof-intel-hda-common.o
 obj-$(CONFIG_SND_SOC_SOF_HDA) += snd-sof-intel-hda.o
+obj-$(CONFIG_SND_SOC_SOF_INTEL_CLIENT) += snd-sof-intel-client.o
diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index 4eeade2e77f7..ce2dcd6aa7de 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -18,6 +18,7 @@
 #include "../sof-priv.h"
 #include "hda.h"
 #include "../sof-audio.h"
+#include "intel-client.h"
 
 static const struct snd_sof_debugfs_map apl_dsp_debugfs[] = {
 	{"hda", HDA_DSP_HDA_BAR, 0, 0x4000, SOF_DEBUGFS_ACCESS_ALWAYS},
@@ -25,6 +26,16 @@ static const struct snd_sof_debugfs_map apl_dsp_debugfs[] = {
 	{"dsp", HDA_DSP_BAR,  0, 0x10000, SOF_DEBUGFS_ACCESS_ALWAYS},
 };
 
+static int apl_register_clients(struct snd_sof_dev *sdev)
+{
+	return intel_register_ipc_test_clients(sdev);
+}
+
+static void apl_unregister_clients(struct snd_sof_dev *sdev)
+{
+	intel_unregister_ipc_test_clients(sdev);
+}
+
 /* apollolake ops */
 const struct snd_sof_dsp_ops sof_apl_ops = {
 	/* probe and remove */
@@ -101,6 +112,10 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
 	.trace_release = hda_dsp_trace_release,
 	.trace_trigger = hda_dsp_trace_trigger,
 
+	/* client ops */
+	.register_clients = apl_register_clients,
+	.unregister_clients = apl_unregister_clients,
+
 	/* DAI drivers */
 	.drv		= skl_dai,
 	.num_drv	= SOF_SKL_NUM_DAIS,
@@ -140,3 +155,4 @@ const struct sof_intel_dsp_desc apl_chip_info = {
 	.ssp_base_offset = APL_SSP_BASE_OFFSET,
 };
 EXPORT_SYMBOL_NS(apl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
+MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_CLIENT);
diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c
index 50a4a73e6b9f..d6fb5e228e93 100644
--- a/sound/soc/sof/intel/bdw.c
+++ b/sound/soc/sof/intel/bdw.c
@@ -18,6 +18,7 @@
 #include "../ops.h"
 #include "shim.h"
 #include "../sof-audio.h"
+#include "intel-client.h"
 
 /* BARs */
 #define BDW_DSP_BAR 0
@@ -563,6 +564,16 @@ static void bdw_set_mach_params(const struct snd_soc_acpi_mach *mach,
 	mach_params->platform = dev_name(dev);
 }
 
+static int bdw_register_clients(struct snd_sof_dev *sdev)
+{
+	return intel_register_ipc_test_clients(sdev);
+}
+
+static void bdw_unregister_clients(struct snd_sof_dev *sdev)
+{
+	intel_unregister_ipc_test_clients(sdev);
+}
+
 /* Broadwell DAIs */
 static struct snd_soc_dai_driver bdw_dai[] = {
 {
@@ -638,6 +649,10 @@ const struct snd_sof_dsp_ops sof_bdw_ops = {
 	/*Firmware loading */
 	.load_firmware	= snd_sof_load_firmware_memcpy,
 
+	/* client ops */
+	.register_clients = bdw_register_clients,
+	.unregister_clients = bdw_unregister_clients,
+
 	/* DAI drivers */
 	.drv = bdw_dai,
 	.num_drv = ARRAY_SIZE(bdw_dai),
@@ -662,3 +677,4 @@ EXPORT_SYMBOL_NS(bdw_chip_info, SND_SOC_SOF_BROADWELL);
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_HIFI_EP_IPC);
 MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA);
+MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_CLIENT);
diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c
index 186736ee5fc2..a42820606ace 100644
--- a/sound/soc/sof/intel/byt.c
+++ b/sound/soc/sof/intel/byt.c
@@ -19,6 +19,7 @@
 #include "shim.h"
 #include "../sof-audio.h"
 #include "../../intel/common/soc-intel-quirks.h"
+#include "intel-client.h"
 
 /* DSP memories */
 #define IRAM_OFFSET		0x0C0000
@@ -821,6 +822,16 @@ static int byt_acpi_probe(struct snd_sof_dev *sdev)
 	return ret;
 }
 
+static int byt_register_clients(struct snd_sof_dev *sdev)
+{
+	return intel_register_ipc_test_clients(sdev);
+}
+
+static void byt_unregister_clients(struct snd_sof_dev *sdev)
+{
+	intel_unregister_ipc_test_clients(sdev);
+}
+
 /* baytrail ops */
 const struct snd_sof_dsp_ops sof_byt_ops = {
 	/* device init */
@@ -879,6 +890,10 @@ const struct snd_sof_dsp_ops sof_byt_ops = {
 	.suspend = byt_suspend,
 	.resume = byt_resume,
 
+	/* client ops */
+	.register_clients = byt_register_clients,
+	.unregister_clients = byt_unregister_clients,
+
 	/* DAI drivers */
 	.drv = byt_dai,
 	.num_drv = 3, /* we have only 3 SSPs on byt*/
@@ -958,6 +973,10 @@ const struct snd_sof_dsp_ops sof_cht_ops = {
 	.suspend = byt_suspend,
 	.resume = byt_resume,
 
+	/* client ops */
+	.register_clients = byt_register_clients,
+	.unregister_clients = byt_unregister_clients,
+
 	/* DAI drivers */
 	.drv = byt_dai,
 	/* all 6 SSPs may be available for cherrytrail */
@@ -985,3 +1004,4 @@ EXPORT_SYMBOL_NS(cht_chip_info, SND_SOC_SOF_BAYTRAIL);
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_HIFI_EP_IPC);
 MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA);
+MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_CLIENT);
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index a5d3258104c0..20afb622c315 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -19,6 +19,7 @@
 #include "hda.h"
 #include "hda-ipc.h"
 #include "../sof-audio.h"
+#include "intel-client.h"
 
 static const struct snd_sof_debugfs_map cnl_dsp_debugfs[] = {
 	{"hda", HDA_DSP_HDA_BAR, 0, 0x4000, SOF_DEBUGFS_ACCESS_ALWAYS},
@@ -230,6 +231,16 @@ void cnl_ipc_dump(struct snd_sof_dev *sdev)
 		hipcida, hipctdr, hipcctl);
 }
 
+static int cnl_register_clients(struct snd_sof_dev *sdev)
+{
+	return intel_register_ipc_test_clients(sdev);
+}
+
+static void cnl_unregister_clients(struct snd_sof_dev *sdev)
+{
+	intel_unregister_ipc_test_clients(sdev);
+}
+
 /* cannonlake ops */
 const struct snd_sof_dsp_ops sof_cnl_ops = {
 	/* probe and remove */
@@ -306,6 +317,10 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
 	.trace_release = hda_dsp_trace_release,
 	.trace_trigger = hda_dsp_trace_trigger,
 
+	/* client ops */
+	.register_clients = cnl_register_clients,
+	.unregister_clients = cnl_unregister_clients,
+
 	/* DAI drivers */
 	.drv		= skl_dai,
 	.num_drv	= SOF_SKL_NUM_DAIS,
@@ -393,3 +408,4 @@ const struct sof_intel_dsp_desc jsl_chip_info = {
 	.ssp_base_offset = CNL_SSP_BASE_OFFSET,
 };
 EXPORT_SYMBOL_NS(jsl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
+MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_CLIENT);
diff --git a/sound/soc/sof/intel/intel-client.c b/sound/soc/sof/intel/intel-client.c
new file mode 100644
index 000000000000..a612de365fc5
--- /dev/null
+++ b/sound/soc/sof/intel/intel-client.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2020 Intel Corporation. All rights reserved.
+//
+// Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
+//
+
+#include <linux/module.h>
+#include "../sof-priv.h"
+#include "../sof-client.h"
+#include "intel-client.h"
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT)
+int intel_register_ipc_test_clients(struct snd_sof_dev *sdev)
+{
+	int ret;
+
+	/*
+	 * Register 2 IPC clients to facilitate tandem flood test. The device name below is
+	 * appended with the device ID assigned automatically when the auxiliary device is
+	 * registered making them unique.
+	 */
+	ret = sof_client_dev_register(sdev, "ipc_test", 0);
+	if (ret < 0)
+		return ret;
+
+	return sof_client_dev_register(sdev, "ipc_test", 1);
+}
+EXPORT_SYMBOL_NS_GPL(intel_register_ipc_test_clients, SND_SOC_SOF_INTEL_CLIENT);
+
+void intel_unregister_ipc_test_clients(struct snd_sof_dev *sdev)
+{
+	sof_client_dev_unregister(sdev, "ipc_test", 0);
+	sof_client_dev_unregister(sdev, "ipc_test", 1);
+}
+EXPORT_SYMBOL_NS_GPL(intel_unregister_ipc_test_clients, SND_SOC_SOF_INTEL_CLIENT);
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SND_SOC_SOF_CLIENT);
diff --git a/sound/soc/sof/intel/intel-client.h b/sound/soc/sof/intel/intel-client.h
new file mode 100644
index 000000000000..49b2c6c0dcc4
--- /dev/null
+++ b/sound/soc/sof/intel/intel-client.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * Copyright(c) 2020 Intel Corporation. All rights reserved.
+ *
+ * Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
+ */
+
+#ifndef __INTEL_CLIENT_H
+#define __INTEL_CLIENT_H
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT)
+int intel_register_ipc_test_clients(struct snd_sof_dev *sdev);
+void intel_unregister_ipc_test_clients(struct snd_sof_dev *sdev);
+#else
+static inline int intel_register_ipc_test_clients(struct snd_sof_dev *sdev)
+{
+	return 0;
+}
+
+static void intel_unregister_ipc_test_clients(struct snd_sof_dev *sdev) {}
+#endif
+
+#endif
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 06/10] ASoC: SOF: Intel: Remove IPC flood test support in SOF core
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (4 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 05/10] ASoC: SOF: Intel: Define " Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 07/10] ASoC: SOF: sof-client: Add client APIs to access probes ops Dave Ertman
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Remove the IPC flood test support in the SOF core as it is
now added in the IPC flood test client.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/Kconfig    |   8 --
 sound/soc/sof/debug.c    | 230 ---------------------------------------
 sound/soc/sof/sof-priv.h |   6 +-
 3 files changed, 1 insertion(+), 243 deletions(-)

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index 13bde36cc5d7..a0f9474b8143 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -182,14 +182,6 @@ config SND_SOC_SOF_DEBUG_ENABLE_FIRMWARE_TRACE
 	  module parameter (similar to dynamic debug)
 	  If unsure, select "N".
 
-config SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST
-	bool "SOF enable IPC flood test"
-	help
-	  This option enables the IPC flood test which can be used to flood
-	  the DSP with test IPCs and gather stats about response times.
-	  Say Y if you want to enable IPC flood test.
-	  If unsure, select "N".
-
 config SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT
 	tristate "SOF enable IPC flood test client"
 	depends on SND_SOC_SOF_CLIENT
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index 9419a99bab53..d224641768da 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -232,120 +232,10 @@ static int snd_sof_debugfs_probe_item(struct snd_sof_dev *sdev,
 }
 #endif
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-#define MAX_IPC_FLOOD_DURATION_MS 1000
-#define MAX_IPC_FLOOD_COUNT 10000
-#define IPC_FLOOD_TEST_RESULT_LEN 512
-
-static int sof_debug_ipc_flood_test(struct snd_sof_dev *sdev,
-				    struct snd_sof_dfsentry *dfse,
-				    bool flood_duration_test,
-				    unsigned long ipc_duration_ms,
-				    unsigned long ipc_count)
-{
-	struct sof_ipc_cmd_hdr hdr;
-	struct sof_ipc_reply reply;
-	u64 min_response_time = U64_MAX;
-	ktime_t start, end, test_end;
-	u64 avg_response_time = 0;
-	u64 max_response_time = 0;
-	u64 ipc_response_time;
-	int i = 0;
-	int ret;
-
-	/* configure test IPC */
-	hdr.cmd = SOF_IPC_GLB_TEST_MSG | SOF_IPC_TEST_IPC_FLOOD;
-	hdr.size = sizeof(hdr);
-
-	/* set test end time for duration flood test */
-	if (flood_duration_test)
-		test_end = ktime_get_ns() + ipc_duration_ms * NSEC_PER_MSEC;
-
-	/* send test IPC's */
-	while (1) {
-		start = ktime_get();
-		ret = sof_ipc_tx_message(sdev->ipc, hdr.cmd, &hdr, hdr.size,
-					 &reply, sizeof(reply));
-		end = ktime_get();
-
-		if (ret < 0)
-			break;
-
-		/* compute min and max response times */
-		ipc_response_time = ktime_to_ns(ktime_sub(end, start));
-		min_response_time = min(min_response_time, ipc_response_time);
-		max_response_time = max(max_response_time, ipc_response_time);
-
-		/* sum up response times */
-		avg_response_time += ipc_response_time;
-		i++;
-
-		/* test complete? */
-		if (flood_duration_test) {
-			if (ktime_to_ns(end) >= test_end)
-				break;
-		} else {
-			if (i == ipc_count)
-				break;
-		}
-	}
-
-	if (ret < 0)
-		dev_err(sdev->dev,
-			"error: ipc flood test failed at %d iterations\n", i);
-
-	/* return if the first IPC fails */
-	if (!i)
-		return ret;
-
-	/* compute average response time */
-	do_div(avg_response_time, i);
-
-	/* clear previous test output */
-	memset(dfse->cache_buf, 0, IPC_FLOOD_TEST_RESULT_LEN);
-
-	if (flood_duration_test) {
-		dev_dbg(sdev->dev, "IPC Flood test duration: %lums\n",
-			ipc_duration_ms);
-		snprintf(dfse->cache_buf, IPC_FLOOD_TEST_RESULT_LEN,
-			 "IPC Flood test duration: %lums\n", ipc_duration_ms);
-	}
-
-	dev_dbg(sdev->dev,
-		"IPC Flood count: %d, Avg response time: %lluns\n",
-		i, avg_response_time);
-	dev_dbg(sdev->dev, "Max response time: %lluns\n",
-		max_response_time);
-	dev_dbg(sdev->dev, "Min response time: %lluns\n",
-		min_response_time);
-
-	/* format output string */
-	snprintf(dfse->cache_buf + strlen(dfse->cache_buf),
-		 IPC_FLOOD_TEST_RESULT_LEN - strlen(dfse->cache_buf),
-		 "IPC Flood count: %d\nAvg response time: %lluns\n",
-		 i, avg_response_time);
-
-	snprintf(dfse->cache_buf + strlen(dfse->cache_buf),
-		 IPC_FLOOD_TEST_RESULT_LEN - strlen(dfse->cache_buf),
-		 "Max response time: %lluns\nMin response time: %lluns\n",
-		 max_response_time, min_response_time);
-
-	return ret;
-}
-#endif
 
 static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
 				  size_t count, loff_t *ppos)
 {
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-	struct snd_sof_dfsentry *dfse = file->private_data;
-	struct snd_sof_dev *sdev = dfse->sdev;
-	unsigned long ipc_duration_ms = 0;
-	bool flood_duration_test = false;
-	unsigned long ipc_count = 0;
-	struct dentry *dentry;
-	int err;
-#endif
 	size_t size;
 	char *string;
 	int ret;
@@ -357,78 +247,6 @@ static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
 	size = simple_write_to_buffer(string, count, ppos, buffer, count);
 	ret = size;
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-	/*
-	 * write op is only supported for ipc_flood_count or
-	 * ipc_flood_duration_ms debugfs entries atm.
-	 * ipc_flood_count floods the DSP with the number of IPC's specified.
-	 * ipc_duration_ms test floods the DSP for the time specified
-	 * in the debugfs entry.
-	 */
-	dentry = file->f_path.dentry;
-	if (strcmp(dentry->d_name.name, "ipc_flood_count") &&
-	    strcmp(dentry->d_name.name, "ipc_flood_duration_ms")) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (!strcmp(dentry->d_name.name, "ipc_flood_duration_ms"))
-		flood_duration_test = true;
-
-	/* test completion criterion */
-	if (flood_duration_test)
-		ret = kstrtoul(string, 0, &ipc_duration_ms);
-	else
-		ret = kstrtoul(string, 0, &ipc_count);
-	if (ret < 0)
-		goto out;
-
-	/* limit max duration/ipc count for flood test */
-	if (flood_duration_test) {
-		if (!ipc_duration_ms) {
-			ret = size;
-			goto out;
-		}
-
-		/* find the minimum. min() is not used to avoid warnings */
-		if (ipc_duration_ms > MAX_IPC_FLOOD_DURATION_MS)
-			ipc_duration_ms = MAX_IPC_FLOOD_DURATION_MS;
-	} else {
-		if (!ipc_count) {
-			ret = size;
-			goto out;
-		}
-
-		/* find the minimum. min() is not used to avoid warnings */
-		if (ipc_count > MAX_IPC_FLOOD_COUNT)
-			ipc_count = MAX_IPC_FLOOD_COUNT;
-	}
-
-	ret = pm_runtime_get_sync(sdev->dev);
-	if (ret < 0 && ret != -EACCES) {
-		dev_err_ratelimited(sdev->dev,
-				    "error: debugfs write failed to resume %d\n",
-				    ret);
-		pm_runtime_put_noidle(sdev->dev);
-		goto out;
-	}
-
-	/* flood test */
-	ret = sof_debug_ipc_flood_test(sdev, dfse, flood_duration_test,
-				       ipc_duration_ms, ipc_count);
-
-	pm_runtime_mark_last_busy(sdev->dev);
-	err = pm_runtime_put_autosuspend(sdev->dev);
-	if (err < 0)
-		dev_err_ratelimited(sdev->dev,
-				    "error: debugfs write failed to idle %d\n",
-				    err);
-
-	/* return size if test is successful */
-	if (ret >= 0)
-		ret = size;
-out:
-#endif
 	kfree(string);
 	return ret;
 }
@@ -444,25 +262,6 @@ static ssize_t sof_dfsentry_read(struct file *file, char __user *buffer,
 	int size;
 	u8 *buf;
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-	struct dentry *dentry;
-
-	dentry = file->f_path.dentry;
-	if ((!strcmp(dentry->d_name.name, "ipc_flood_count") ||
-	     !strcmp(dentry->d_name.name, "ipc_flood_duration_ms")) &&
-	    dfse->cache_buf) {
-		if (*ppos)
-			return 0;
-
-		count = strlen(dfse->cache_buf);
-		size_ret = copy_to_user(buffer, dfse->cache_buf, count);
-		if (size_ret)
-			return -EFAULT;
-
-		*ppos += count;
-		return count;
-	}
-#endif
 	size = dfse->size;
 
 	/* validate position & count */
@@ -606,17 +405,6 @@ int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev,
 	dfse->size = size;
 	dfse->sdev = sdev;
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-	/*
-	 * cache_buf is unused for SOF_DFSENTRY_TYPE_BUF debugfs entries.
-	 * So, use it to save the results of the last IPC flood test.
-	 */
-	dfse->cache_buf = devm_kzalloc(sdev->dev, IPC_FLOOD_TEST_RESULT_LEN,
-				       GFP_KERNEL);
-	if (!dfse->cache_buf)
-		return -ENOMEM;
-#endif
-
 	debugfs_create_file(name, mode, sdev->debugfs_root, dfse,
 			    &sof_dfs_fops);
 	/* add to dfsentry list */
@@ -662,24 +450,6 @@ int snd_sof_dbg_init(struct snd_sof_dev *sdev)
 		return err;
 #endif
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
-	/* create read-write ipc_flood_count debugfs entry */
-	err = snd_sof_debugfs_buf_item(sdev, NULL, 0,
-				       "ipc_flood_count", 0666);
-
-	/* errors are only due to memory allocation, not debugfs */
-	if (err < 0)
-		return err;
-
-	/* create read-write ipc_flood_duration_ms debugfs entry */
-	err = snd_sof_debugfs_buf_item(sdev, NULL, 0,
-				       "ipc_flood_duration_ms", 0666);
-
-	/* errors are only due to memory allocation, not debugfs */
-	if (err < 0)
-		return err;
-#endif
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_sof_dbg_init);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index cca239c09d0e..e44158410b24 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -50,10 +50,6 @@ extern int sof_core_debug;
 #define SOF_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | \
 	SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_FLOAT)
 
-#define ENABLE_DEBUGFS_CACHEBUF \
-	(IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) || \
-	 IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST))
-
 /* So far the primary core on all DSPs has ID 0 */
 #define SOF_DSP_PRIMARY_CORE 0
 
@@ -301,7 +297,7 @@ struct snd_sof_dfsentry {
 	 * or if it is accessible only when the DSP is in D0.
 	 */
 	enum sof_debugfs_access_type access_type;
-#if ENABLE_DEBUGFS_CACHEBUF
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE)
 	char *cache_buf; /* buffer to cache the contents of debugfs memory */
 #endif
 	struct snd_sof_dev *sdev;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 07/10] ASoC: SOF: sof-client: Add client APIs to access probes ops
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (5 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 06/10] ASoC: SOF: Intel: Remove IPC flood test support in SOF core Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 08/10] ASoC: SOF: compress: move and export sof_probe_compr_ops Dave Ertman
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add client APIs to invoke the platform-specific DSP probes
ops. Also, add a new API to get the SOF core device pointer
which will be used for DMA buffer allocation.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/sof-client.c | 55 ++++++++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-client.h | 25 +++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/sound/soc/sof/sof-client.c b/sound/soc/sof/sof-client.c
index dd75a0ba4c28..838aaa5ea179 100644
--- a/sound/soc/sof/sof-client.c
+++ b/sound/soc/sof/sof-client.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include "ops.h"
 #include "sof-client.h"
 #include "sof-priv.h"
 
@@ -112,4 +113,58 @@ struct dentry *sof_client_get_debugfs_root(struct sof_client_dev *cdev)
 }
 EXPORT_SYMBOL_NS_GPL(sof_client_get_debugfs_root, SND_SOC_SOF_CLIENT);
 
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES_CLIENT)
+int sof_client_probe_compr_assign(struct sof_client_dev *cdev,
+				  struct snd_compr_stream *cstream,
+				  struct snd_soc_dai *dai)
+{
+	return snd_sof_probe_compr_assign(cdev->sdev, cstream, dai);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_probe_compr_assign, SND_SOC_SOF_CLIENT);
+
+int sof_client_probe_compr_free(struct sof_client_dev *cdev,
+				struct snd_compr_stream *cstream,
+				struct snd_soc_dai *dai)
+{
+	return snd_sof_probe_compr_free(cdev->sdev, cstream, dai);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_probe_compr_free, SND_SOC_SOF_CLIENT);
+
+int sof_client_probe_compr_set_params(struct sof_client_dev *cdev,
+				      struct snd_compr_stream *cstream,
+				      struct snd_compr_params *params,
+				      struct snd_soc_dai *dai)
+{
+	return snd_sof_probe_compr_set_params(cdev->sdev, cstream, params, dai);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_probe_compr_set_params, SND_SOC_SOF_CLIENT);
+
+int sof_client_probe_compr_trigger(struct sof_client_dev *cdev,
+				   struct snd_compr_stream *cstream, int cmd,
+				   struct snd_soc_dai *dai)
+{
+	return snd_sof_probe_compr_trigger(cdev->sdev, cstream, cmd, dai);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_probe_compr_trigger, SND_SOC_SOF_CLIENT);
+
+int sof_client_probe_compr_pointer(struct sof_client_dev *cdev,
+				   struct snd_compr_stream *cstream,
+				   struct snd_compr_tstamp *tstamp,
+				   struct snd_soc_dai *dai)
+{
+	return snd_sof_probe_compr_pointer(cdev->sdev, cstream, tstamp, dai);
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_probe_compr_pointer, SND_SOC_SOF_CLIENT);
+#endif
+
+/*
+ * DMA buffer alloc fails when using the client device. Use the SOF core device instead.
+ * This will be needed for clients other than the probes client device as well.
+ */
+struct device *sof_client_get_dma_dev(struct sof_client_dev *cdev)
+{
+	return cdev->sdev->dev;
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_get_dma_dev, SND_SOC_SOF_CLIENT);
+
 MODULE_LICENSE("GPL");
diff --git a/sound/soc/sof/sof-client.h b/sound/soc/sof/sof-client.h
index 429282df9f65..be80053068c9 100644
--- a/sound/soc/sof/sof-client.h
+++ b/sound/soc/sof/sof-client.h
@@ -7,6 +7,10 @@
 #include <linux/device.h>
 #include <linux/idr.h>
 #include <linux/list.h>
+#include <sound/compress_offload.h>
+#include <sound/compress_driver.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
 
 #define SOF_CLIENT_PROBE_TIMEOUT_MS 2000
 
@@ -50,6 +54,27 @@ int sof_client_ipc_tx_message(struct sof_client_dev *cdev, u32 header, void *msg
 			      size_t msg_bytes, void *reply_data, size_t reply_bytes);
 
 struct dentry *sof_client_get_debugfs_root(struct sof_client_dev *cdev);
+struct device *sof_client_get_dma_dev(struct sof_client_dev *cdev);
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES_CLIENT)
+int sof_client_probe_compr_assign(struct sof_client_dev *cdev,
+				  struct snd_compr_stream *cstream,
+				  struct snd_soc_dai *dai);
+int sof_client_probe_compr_free(struct sof_client_dev *cdev,
+				struct snd_compr_stream *cstream,
+				struct snd_soc_dai *dai);
+int sof_client_probe_compr_set_params(struct sof_client_dev *cdev,
+				      struct snd_compr_stream *cstream,
+				      struct snd_compr_params *params,
+				      struct snd_soc_dai *dai);
+int sof_client_probe_compr_trigger(struct sof_client_dev *cdev,
+				   struct snd_compr_stream *cstream, int cmd,
+				   struct snd_soc_dai *dai);
+int sof_client_probe_compr_pointer(struct sof_client_dev *cdev,
+				   struct snd_compr_stream *cstream,
+				   struct snd_compr_tstamp *tstamp,
+				   struct snd_soc_dai *dai);
+#endif
 
 /**
  * module_sof_client_driver() - Helper macro for registering an SOF Client
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 08/10] ASoC: SOF: compress: move and export sof_probe_compr_ops
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (6 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 07/10] ASoC: SOF: sof-client: Add client APIs to access probes ops Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 09/10] ASoC: SOF: Add new client driver for probes support Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 10/10] ASoC: SOF: Intel: CNL: register probes client Dave Ertman
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

sof_probe_compr_ops are not platform-specific. So move
it to common compress code and export the symbol. The
compilation of the common compress code is already dependent
on the selection of CONFIG_SND_SOC_SOF_DEBUG_PROBES, so no
need to check the Kconfig section for defining sof_probe_compr_ops
again.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/compress.c      |  9 +++++++++
 sound/soc/sof/compress.h      |  1 +
 sound/soc/sof/intel/hda-dai.c | 12 ------------
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c
index 2d4969c705a4..0443f171b4e7 100644
--- a/sound/soc/sof/compress.c
+++ b/sound/soc/sof/compress.c
@@ -13,6 +13,15 @@
 #include "ops.h"
 #include "probe.h"
 
+struct snd_soc_cdai_ops sof_probe_compr_ops = {
+	.startup	= sof_probe_compr_open,
+	.shutdown	= sof_probe_compr_free,
+	.set_params	= sof_probe_compr_set_params,
+	.trigger	= sof_probe_compr_trigger,
+	.pointer	= sof_probe_compr_pointer,
+};
+EXPORT_SYMBOL(sof_probe_compr_ops);
+
 struct snd_compress_ops sof_probe_compressed_ops = {
 	.copy		= sof_probe_compr_copy,
 };
diff --git a/sound/soc/sof/compress.h b/sound/soc/sof/compress.h
index ca8790bd4b13..689c83ac8ffc 100644
--- a/sound/soc/sof/compress.h
+++ b/sound/soc/sof/compress.h
@@ -13,6 +13,7 @@
 
 #include <sound/compress_driver.h>
 
+extern struct snd_soc_cdai_ops sof_probe_compr_ops;
 extern struct snd_compress_ops sof_probe_compressed_ops;
 
 int sof_probe_compr_open(struct snd_compr_stream *cstream,
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index c6cb8c212eca..1acec1176986 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -400,18 +400,6 @@ static const struct snd_soc_dai_ops hda_link_dai_ops = {
 	.prepare = hda_link_pcm_prepare,
 };
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
-#include "../compress.h"
-
-static struct snd_soc_cdai_ops sof_probe_compr_ops = {
-	.startup	= sof_probe_compr_open,
-	.shutdown	= sof_probe_compr_free,
-	.set_params	= sof_probe_compr_set_params,
-	.trigger	= sof_probe_compr_trigger,
-	.pointer	= sof_probe_compr_pointer,
-};
-
-#endif
 #endif
 
 /*
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 09/10] ASoC: SOF: Add new client driver for probes support
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (7 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 08/10] ASoC: SOF: compress: move and export sof_probe_compr_ops Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  2020-11-13 16:18 ` [PATCH v4 10/10] ASoC: SOF: Intel: CNL: register probes client Dave Ertman
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add a new client driver for probes support and move
all the probes-related code from the core to the
client driver.

The probes client driver registers a component driver
with one CPU DAI driver for extraction and creates a
new sound card with one DUMMY DAI link with a dummy codec
that will be used for extracting audio data from specific
points in the audio pipeline.

The probes debugfs ops are based on the initial
implementation by Cezary Rojewski and have been moved
out of the SOF core into the client driver making it
easier to maintain. This change will make it easier
for the probes functionality to be added for all platforms
without having the need to modify the existing(15+) machine
drivers.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/Kconfig             |  18 +-
 sound/soc/sof/Makefile            |   3 +-
 sound/soc/sof/compress.c          |  51 ++--
 sound/soc/sof/core.c              |   6 -
 sound/soc/sof/debug.c             | 227 ----------------
 sound/soc/sof/intel/hda-dai.c     |  15 --
 sound/soc/sof/intel/hda.h         |   6 -
 sound/soc/sof/pcm.c               |  11 -
 sound/soc/sof/probe.c             | 124 ++++-----
 sound/soc/sof/probe.h             |  41 +--
 sound/soc/sof/sof-priv.h          |   4 -
 sound/soc/sof/sof-probes-client.c | 414 ++++++++++++++++++++++++++++++
 12 files changed, 545 insertions(+), 375 deletions(-)
 create mode 100644 sound/soc/sof/sof-probes-client.c

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index a0f9474b8143..9fa00780c842 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -42,13 +42,11 @@ config SND_SOC_SOF_OF
 	  Say Y if you need this option. If unsure select "N".
 
 config SND_SOC_SOF_DEBUG_PROBES
-	bool "SOF enable data probing"
+	bool
 	select SND_SOC_COMPRESS
 	help
-	  This option enables the data probing feature that can be used to
-	  gather data directly from specific points of the audio pipeline.
-	  Say Y if you want to enable probes.
-	  If unsure, select "N".
+	  This option is not user-selectable but automagically handled by
+	  'select' statements at a higher level.
 
 config SND_SOC_SOF_CLIENT
 	tristate
@@ -192,6 +190,15 @@ config SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT
 	  Say Y if you want to enable IPC flood test.
 	  If unsure, select "N".
 
+config SND_SOC_SOF_DEBUG_PROBES_CLIENT
+	tristate "SOF enable data probing"
+	depends on SND_SOC_SOF_CLIENT
+	help
+	  This option enables the data probing feature that can be used to
+	  gather data directly from specific points of the audio pipeline.
+	  Say Y if you want to enable probes.
+	  If unsure, select "N".
+
 config SND_SOC_SOF_DEBUG_RETAIN_DSP_CONTEXT
 	bool "SOF retain DSP context on any FW exceptions"
 	help
@@ -207,6 +214,7 @@ endif ## SND_SOC_SOF_DEVELOPER_SUPPORT
 config SND_SOC_SOF
 	tristate
 	select SND_SOC_SOF_CLIENT if SND_SOC_SOF_CLIENT_SUPPORT
+	select SND_SOC_SOF_DEBUG_PROBES if SND_SOC_SOF_DEBUG_PROBES_CLIENT
 	select SND_SOC_TOPOLOGY
 	select SND_SOC_SOF_NOCODEC if SND_SOC_SOF_NOCODEC_SUPPORT
 	help
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index baa93fe2cc9a..cf49466f7910 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -3,7 +3,7 @@
 snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
 		control.o trace.o utils.o sof-audio.o
 snd-sof-client-objs := sof-client.o
-snd-sof-$(CONFIG_SND_SOC_SOF_DEBUG_PROBES) += probe.o compress.o
+snd-sof-probes-objs := probe.o compress.o sof-probes-client.o
 
 snd-sof-pci-objs := sof-pci-dev.o
 snd-sof-acpi-objs := sof-acpi-dev.o
@@ -24,6 +24,7 @@ obj-$(CONFIG_SND_SOC_SOF_PCI) += snd-sof-pci.o
 obj-$(CONFIG_SND_SOC_SOF_CLIENT) += snd-sof-client.o
 
 obj-$(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST_CLIENT) += snd-sof-ipc-test.o
+obj-$(CONFIG_SND_SOC_SOF_DEBUG_PROBES_CLIENT) += snd-sof-probes.o
 
 obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/
 obj-$(CONFIG_SND_SOC_SOF_IMX_TOPLEVEL) += imx/
diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c
index 0443f171b4e7..bbb77f028e74 100644
--- a/sound/soc/sof/compress.c
+++ b/sound/soc/sof/compress.c
@@ -10,8 +10,8 @@
 
 #include <sound/soc.h>
 #include "compress.h"
-#include "ops.h"
 #include "probe.h"
+#include "sof-client.h"
 
 struct snd_soc_cdai_ops sof_probe_compr_ops = {
 	.startup	= sof_probe_compr_open,
@@ -30,17 +30,18 @@ EXPORT_SYMBOL(sof_probe_compressed_ops);
 int sof_probe_compr_open(struct snd_compr_stream *cstream,
 		struct snd_soc_dai *dai)
 {
-	struct snd_sof_dev *sdev =
-				snd_soc_component_get_drvdata(dai->component);
+	struct snd_soc_card *card = snd_soc_component_get_drvdata(dai->component);
+	struct sof_client_dev *cdev = snd_soc_card_get_drvdata(card);
+	struct sof_probes_data *probes_data = cdev->data;
 	int ret;
 
-	ret = snd_sof_probe_compr_assign(sdev, cstream, dai);
+	ret = sof_client_probe_compr_assign(cdev, cstream, dai);
 	if (ret < 0) {
 		dev_err(dai->dev, "Failed to assign probe stream: %d\n", ret);
 		return ret;
 	}
 
-	sdev->extractor_stream_tag = ret;
+	probes_data->extractor_stream_tag = ret;
 	return 0;
 }
 EXPORT_SYMBOL(sof_probe_compr_open);
@@ -48,55 +49,57 @@ EXPORT_SYMBOL(sof_probe_compr_open);
 int sof_probe_compr_free(struct snd_compr_stream *cstream,
 		struct snd_soc_dai *dai)
 {
-	struct snd_sof_dev *sdev =
-				snd_soc_component_get_drvdata(dai->component);
+	struct snd_soc_card *card = snd_soc_component_get_drvdata(dai->component);
+	struct sof_client_dev *cdev = snd_soc_card_get_drvdata(card);
+	struct sof_probes_data *probes_data = cdev->data;
 	struct sof_probe_point_desc *desc;
 	size_t num_desc;
 	int i, ret;
 
 	/* disconnect all probe points */
-	ret = sof_ipc_probe_points_info(sdev, &desc, &num_desc);
+	ret = sof_probe_points_info(cdev, &desc, &num_desc);
 	if (ret < 0) {
 		dev_err(dai->dev, "Failed to get probe points: %d\n", ret);
 		goto exit;
 	}
 
 	for (i = 0; i < num_desc; i++)
-		sof_ipc_probe_points_remove(sdev, &desc[i].buffer_id, 1);
+		sof_probe_points_remove(cdev, &desc[i].buffer_id, 1);
 	kfree(desc);
 
 exit:
-	ret = sof_ipc_probe_deinit(sdev);
+	ret = sof_probe_deinit(cdev);
 	if (ret < 0)
 		dev_err(dai->dev, "Failed to deinit probe: %d\n", ret);
 
-	sdev->extractor_stream_tag = SOF_PROBE_INVALID_NODE_ID;
+	probes_data->extractor_stream_tag = SOF_PROBE_INVALID_NODE_ID;
 	snd_compr_free_pages(cstream);
 
-	return snd_sof_probe_compr_free(sdev, cstream, dai);
+	return sof_client_probe_compr_free(cdev, cstream, dai);
 }
 EXPORT_SYMBOL(sof_probe_compr_free);
 
 int sof_probe_compr_set_params(struct snd_compr_stream *cstream,
 		struct snd_compr_params *params, struct snd_soc_dai *dai)
 {
+	struct snd_soc_card *card = snd_soc_component_get_drvdata(dai->component);
+	struct sof_client_dev *cdev = snd_soc_card_get_drvdata(card);
+	struct sof_probes_data *probes_data = cdev->data;
 	struct snd_compr_runtime *rtd = cstream->runtime;
-	struct snd_sof_dev *sdev =
-				snd_soc_component_get_drvdata(dai->component);
 	int ret;
 
 	cstream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_SG;
-	cstream->dma_buffer.dev.dev = sdev->dev;
+	cstream->dma_buffer.dev.dev = sof_client_get_dma_dev(cdev);
 	ret = snd_compr_malloc_pages(cstream, rtd->buffer_size);
 	if (ret < 0)
 		return ret;
 
-	ret = snd_sof_probe_compr_set_params(sdev, cstream, params, dai);
+	ret = sof_client_probe_compr_set_params(cdev, cstream, params, dai);
 	if (ret < 0)
 		return ret;
 
-	ret = sof_ipc_probe_init(sdev, sdev->extractor_stream_tag,
-				 rtd->dma_bytes);
+	ret = sof_probe_init(cdev, probes_data->extractor_stream_tag,
+			     rtd->dma_bytes);
 	if (ret < 0) {
 		dev_err(dai->dev, "Failed to init probe: %d\n", ret);
 		return ret;
@@ -109,20 +112,20 @@ EXPORT_SYMBOL(sof_probe_compr_set_params);
 int sof_probe_compr_trigger(struct snd_compr_stream *cstream, int cmd,
 		struct snd_soc_dai *dai)
 {
-	struct snd_sof_dev *sdev =
-				snd_soc_component_get_drvdata(dai->component);
+	struct snd_soc_card *card = snd_soc_component_get_drvdata(dai->component);
+	struct sof_client_dev *cdev = snd_soc_card_get_drvdata(card);
 
-	return snd_sof_probe_compr_trigger(sdev, cstream, cmd, dai);
+	return sof_client_probe_compr_trigger(cdev, cstream, cmd, dai);
 }
 EXPORT_SYMBOL(sof_probe_compr_trigger);
 
 int sof_probe_compr_pointer(struct snd_compr_stream *cstream,
 		struct snd_compr_tstamp *tstamp, struct snd_soc_dai *dai)
 {
-	struct snd_sof_dev *sdev =
-				snd_soc_component_get_drvdata(dai->component);
+	struct snd_soc_card *card = snd_soc_component_get_drvdata(dai->component);
+	struct sof_client_dev *cdev = snd_soc_card_get_drvdata(card);
 
-	return snd_sof_probe_compr_pointer(sdev, cstream, tstamp, dai);
+	return sof_client_probe_compr_pointer(cdev, cstream, tstamp, dai);
 }
 EXPORT_SYMBOL(sof_probe_compr_pointer);
 
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index ddb9a12d5aac..9b72317d6525 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -14,9 +14,6 @@
 #include <sound/sof.h>
 #include "sof-priv.h"
 #include "ops.h"
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-#include "probe.h"
-#endif
 
 /* see SOF_DBG_ flags */
 int sof_core_debug;
@@ -305,9 +302,6 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
 	sdev->pdata = plat_data;
 	sdev->first_boot = true;
 	sdev->fw_state = SOF_FW_BOOT_NOT_STARTED;
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-	sdev->extractor_stream_tag = SOF_PROBE_INVALID_NODE_ID;
-#endif
 	dev_set_drvdata(dev, sdev);
 
 	/* check all mandatory ops */
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index d224641768da..234ae704c001 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -17,222 +17,6 @@
 #include "sof-priv.h"
 #include "ops.h"
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-#include "probe.h"
-
-/**
- * strsplit_u32 - Split string into sequence of u32 tokens
- * @buf:	String to split into tokens.
- * @delim:	String containing delimiter characters.
- * @tkns:	Returned u32 sequence pointer.
- * @num_tkns:	Returned number of tokens obtained.
- */
-static int
-strsplit_u32(char **buf, const char *delim, u32 **tkns, size_t *num_tkns)
-{
-	char *s;
-	u32 *data, *tmp;
-	size_t count = 0;
-	size_t cap = 32;
-	int ret = 0;
-
-	*tkns = NULL;
-	*num_tkns = 0;
-	data = kcalloc(cap, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	while ((s = strsep(buf, delim)) != NULL) {
-		ret = kstrtouint(s, 0, data + count);
-		if (ret)
-			goto exit;
-		if (++count >= cap) {
-			cap *= 2;
-			tmp = krealloc(data, cap * sizeof(*data), GFP_KERNEL);
-			if (!tmp) {
-				ret = -ENOMEM;
-				goto exit;
-			}
-			data = tmp;
-		}
-	}
-
-	if (!count)
-		goto exit;
-	*tkns = kmemdup(data, count * sizeof(*data), GFP_KERNEL);
-	if (*tkns == NULL) {
-		ret = -ENOMEM;
-		goto exit;
-	}
-	*num_tkns = count;
-
-exit:
-	kfree(data);
-	return ret;
-}
-
-static int tokenize_input(const char __user *from, size_t count,
-		loff_t *ppos, u32 **tkns, size_t *num_tkns)
-{
-	char *buf;
-	int ret;
-
-	buf = kmalloc(count + 1, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = simple_write_to_buffer(buf, count, ppos, from, count);
-	if (ret != count) {
-		ret = ret >= 0 ? -EIO : ret;
-		goto exit;
-	}
-
-	buf[count] = '\0';
-	ret = strsplit_u32((char **)&buf, ",", tkns, num_tkns);
-exit:
-	kfree(buf);
-	return ret;
-}
-
-static ssize_t probe_points_read(struct file *file,
-		char __user *to, size_t count, loff_t *ppos)
-{
-	struct snd_sof_dfsentry *dfse = file->private_data;
-	struct snd_sof_dev *sdev = dfse->sdev;
-	struct sof_probe_point_desc *desc;
-	size_t num_desc, len = 0;
-	char *buf;
-	int i, ret;
-
-	if (sdev->extractor_stream_tag == SOF_PROBE_INVALID_NODE_ID) {
-		dev_warn(sdev->dev, "no extractor stream running\n");
-		return -ENOENT;
-	}
-
-	buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = sof_ipc_probe_points_info(sdev, &desc, &num_desc);
-	if (ret < 0)
-		goto exit;
-
-	for (i = 0; i < num_desc; i++) {
-		ret = snprintf(buf + len, PAGE_SIZE - len,
-			"Id: %#010x  Purpose: %d  Node id: %#x\n",
-			desc[i].buffer_id, desc[i].purpose, desc[i].stream_tag);
-		if (ret < 0)
-			goto free_desc;
-		len += ret;
-	}
-
-	ret = simple_read_from_buffer(to, count, ppos, buf, len);
-free_desc:
-	kfree(desc);
-exit:
-	kfree(buf);
-	return ret;
-}
-
-static ssize_t probe_points_write(struct file *file,
-		const char __user *from, size_t count, loff_t *ppos)
-{
-	struct snd_sof_dfsentry *dfse = file->private_data;
-	struct snd_sof_dev *sdev = dfse->sdev;
-	struct sof_probe_point_desc *desc;
-	size_t num_tkns, bytes;
-	u32 *tkns;
-	int ret;
-
-	if (sdev->extractor_stream_tag == SOF_PROBE_INVALID_NODE_ID) {
-		dev_warn(sdev->dev, "no extractor stream running\n");
-		return -ENOENT;
-	}
-
-	ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
-	if (ret < 0)
-		return ret;
-	bytes = sizeof(*tkns) * num_tkns;
-	if (!num_tkns || (bytes % sizeof(*desc))) {
-		ret = -EINVAL;
-		goto exit;
-	}
-
-	desc = (struct sof_probe_point_desc *)tkns;
-	ret = sof_ipc_probe_points_add(sdev,
-			desc, bytes / sizeof(*desc));
-	if (!ret)
-		ret = count;
-exit:
-	kfree(tkns);
-	return ret;
-}
-
-static const struct file_operations probe_points_fops = {
-	.open = simple_open,
-	.read = probe_points_read,
-	.write = probe_points_write,
-	.llseek = default_llseek,
-};
-
-static ssize_t probe_points_remove_write(struct file *file,
-		const char __user *from, size_t count, loff_t *ppos)
-{
-	struct snd_sof_dfsentry *dfse = file->private_data;
-	struct snd_sof_dev *sdev = dfse->sdev;
-	size_t num_tkns;
-	u32 *tkns;
-	int ret;
-
-	if (sdev->extractor_stream_tag == SOF_PROBE_INVALID_NODE_ID) {
-		dev_warn(sdev->dev, "no extractor stream running\n");
-		return -ENOENT;
-	}
-
-	ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
-	if (ret < 0)
-		return ret;
-	if (!num_tkns) {
-		ret = -EINVAL;
-		goto exit;
-	}
-
-	ret = sof_ipc_probe_points_remove(sdev, tkns, num_tkns);
-	if (!ret)
-		ret = count;
-exit:
-	kfree(tkns);
-	return ret;
-}
-
-static const struct file_operations probe_points_remove_fops = {
-	.open = simple_open,
-	.write = probe_points_remove_write,
-	.llseek = default_llseek,
-};
-
-static int snd_sof_debugfs_probe_item(struct snd_sof_dev *sdev,
-				 const char *name, mode_t mode,
-				 const struct file_operations *fops)
-{
-	struct snd_sof_dfsentry *dfse;
-
-	dfse = devm_kzalloc(sdev->dev, sizeof(*dfse), GFP_KERNEL);
-	if (!dfse)
-		return -ENOMEM;
-
-	dfse->type = SOF_DFSENTRY_TYPE_BUF;
-	dfse->sdev = sdev;
-
-	debugfs_create_file(name, mode, sdev->debugfs_root, dfse, fops);
-	/* add to dfsentry list */
-	list_add(&dfse->list, &sdev->dfsentry_list);
-
-	return 0;
-}
-#endif
-
-
 static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
 				  size_t count, loff_t *ppos)
 {
@@ -439,17 +223,6 @@ int snd_sof_dbg_init(struct snd_sof_dev *sdev)
 			return err;
 	}
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-	err = snd_sof_debugfs_probe_item(sdev, "probe_points",
-			0644, &probe_points_fops);
-	if (err < 0)
-		return err;
-	err = snd_sof_debugfs_probe_item(sdev, "probe_points_remove",
-			0200, &probe_points_remove_fops);
-	if (err < 0)
-		return err;
-#endif
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_sof_dbg_init);
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 1acec1176986..35965e2e72de 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -557,20 +557,5 @@ struct snd_soc_dai_driver skl_dai[] = {
 		.channels_max = 16,
 	},
 },
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
-{
-	.name = "Probe Extraction CPU DAI",
-	.compress_new = snd_soc_new_compress,
-	.cops = &sof_probe_compr_ops,
-	.capture = {
-		.stream_name = "Probe Extraction",
-		.channels_min = 1,
-		.channels_max = 8,
-		.rates = SNDRV_PCM_RATE_48000,
-		.rate_min = 48000,
-		.rate_max = 48000,
-	},
-},
-#endif
 #endif
 };
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 1bc4dabdd394..f0ac57e8b243 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -352,13 +352,7 @@
 
 /* Number of DAIs */
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
-
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
-#define SOF_SKL_NUM_DAIS		16
-#else
 #define SOF_SKL_NUM_DAIS		15
-#endif
-
 #else
 #define SOF_SKL_NUM_DAIS		8
 #endif
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index cbac6f17c52f..609583f79aa8 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -16,9 +16,6 @@
 #include "sof-priv.h"
 #include "sof-audio.h"
 #include "ops.h"
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-#include "compress.h"
-#endif
 
 /* Create DMA buffer page table for DSP */
 static int create_page_table(struct snd_soc_component *component,
@@ -801,14 +798,6 @@ void snd_sof_new_platform_drv(struct snd_sof_dev *sdev)
 	pd->hw_free = sof_pcm_hw_free;
 	pd->trigger = sof_pcm_trigger;
 	pd->pointer = sof_pcm_pointer;
-
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
-	pd->compress_ops = &sof_compressed_ops;
-#endif
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-	/* override cops when probe support is enabled */
-	pd->compress_ops = &sof_probe_compressed_ops;
-#endif
 	pd->pcm_construct = sof_pcm_new;
 	pd->ignore_machine = drv_name;
 	pd->be_hw_params_fixup = sof_pcm_dai_link_fixup;
diff --git a/sound/soc/sof/probe.c b/sound/soc/sof/probe.c
index 14509f4d3f86..a926619b484a 100644
--- a/sound/soc/sof/probe.c
+++ b/sound/soc/sof/probe.c
@@ -8,12 +8,15 @@
 // Author: Cezary Rojewski <cezary.rojewski@intel.com>
 //
 
-#include "sof-priv.h"
+#include <linux/slab.h>
+#include <sound/sof/header.h>
 #include "probe.h"
+#include "sof-client.h"
+
 
 /**
- * sof_ipc_probe_init - initialize data probing
- * @sdev:		SOF sound device
+ * sof_probe_init - initialize data probing
+ * @cdev:		SOF client device
  * @stream_tag:		Extractor stream tag
  * @buffer_size:	DMA buffer size to set for extractor
  *
@@ -25,8 +28,8 @@
  * Probing is initialized only once and each INIT request must be
  * matched by DEINIT call.
  */
-int sof_ipc_probe_init(struct snd_sof_dev *sdev,
-		u32 stream_tag, size_t buffer_size)
+int sof_probe_init(struct sof_client_dev *cdev, u32 stream_tag,
+		   size_t buffer_size)
 {
 	struct sof_ipc_probe_dma_add_params *msg;
 	struct sof_ipc_reply reply;
@@ -42,22 +45,22 @@ int sof_ipc_probe_init(struct snd_sof_dev *sdev,
 	msg->dma[0].stream_tag = stream_tag;
 	msg->dma[0].dma_buffer_size = buffer_size;
 
-	ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
-			&reply, sizeof(reply));
+	ret = sof_client_ipc_tx_message(cdev, msg->hdr.cmd, msg, msg->hdr.size,
+					&reply, sizeof(reply));
 	kfree(msg);
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_probe_init);
+EXPORT_SYMBOL(sof_probe_init);
 
 /**
- * sof_ipc_probe_deinit - cleanup after data probing
- * @sdev:	SOF sound device
+ * sof_probe_deinit - cleanup after data probing
+ * @cdev:		SOF client device
  *
  * Host sends DEINIT request to free previously initialized probe
  * on DSP side once it is no longer needed. DEINIT only when there
  * are no probes connected and with all injectors detached.
  */
-int sof_ipc_probe_deinit(struct snd_sof_dev *sdev)
+int sof_probe_deinit(struct sof_client_dev *cdev)
 {
 	struct sof_ipc_cmd_hdr msg;
 	struct sof_ipc_reply reply;
@@ -65,13 +68,13 @@ int sof_ipc_probe_deinit(struct snd_sof_dev *sdev)
 	msg.size = sizeof(msg);
 	msg.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_DEINIT;
 
-	return sof_ipc_tx_message(sdev->ipc, msg.cmd, &msg, msg.size,
-			&reply, sizeof(reply));
+	return sof_client_ipc_tx_message(cdev, msg.cmd, &msg, msg.size,
+					 &reply, sizeof(reply));
 }
-EXPORT_SYMBOL(sof_ipc_probe_deinit);
+EXPORT_SYMBOL(sof_probe_deinit);
 
-static int sof_ipc_probe_info(struct snd_sof_dev *sdev, unsigned int cmd,
-		void **params, size_t *num_params)
+static int sof_probe_info(struct sof_client_dev *cdev, unsigned int cmd,
+			  void **params, size_t *num_params)
 {
 	struct sof_ipc_probe_info_params msg = {{{0}}};
 	struct sof_ipc_probe_info_params *reply;
@@ -87,8 +90,9 @@ static int sof_ipc_probe_info(struct snd_sof_dev *sdev, unsigned int cmd,
 	msg.rhdr.hdr.size = sizeof(msg);
 	msg.rhdr.hdr.cmd = SOF_IPC_GLB_PROBE | cmd;
 
-	ret = sof_ipc_tx_message(sdev->ipc, msg.rhdr.hdr.cmd, &msg,
-			msg.rhdr.hdr.size, reply, SOF_IPC_MSG_MAX_SIZE);
+	ret = sof_client_ipc_tx_message(cdev, msg.rhdr.hdr.cmd, &msg,
+					msg.rhdr.hdr.size, reply,
+					SOF_IPC_MSG_MAX_SIZE);
 	if (ret < 0 || reply->rhdr.error < 0)
 		goto exit;
 
@@ -113,8 +117,8 @@ static int sof_ipc_probe_info(struct snd_sof_dev *sdev, unsigned int cmd,
 }
 
 /**
- * sof_ipc_probe_dma_info - retrieve list of active injection dmas
- * @sdev:	SOF sound device
+ * sof_probe_dma_info - retrieve list of active injection dmas
+ * @cdev:		SOF client device
  * @dma:	Returned list of active dmas
  * @num_dma:	Returned count of active dmas
  *
@@ -127,17 +131,17 @@ static int sof_ipc_probe_info(struct snd_sof_dev *sdev, unsigned int cmd,
  * which is not the case for injection where multiple streams
  * could be engaged.
  */
-int sof_ipc_probe_dma_info(struct snd_sof_dev *sdev,
-		struct sof_probe_dma **dma, size_t *num_dma)
+int sof_probe_dma_info(struct sof_client_dev *cdev,
+		       struct sof_probe_dma **dma, size_t *num_dma)
 {
-	return sof_ipc_probe_info(sdev, SOF_IPC_PROBE_DMA_INFO,
-			(void **)dma, num_dma);
+	return sof_probe_info(cdev, SOF_IPC_PROBE_DMA_INFO,
+				  (void **)dma, num_dma);
 }
-EXPORT_SYMBOL(sof_ipc_probe_dma_info);
+EXPORT_SYMBOL(sof_probe_dma_info);
 
 /**
- * sof_ipc_probe_dma_add - attach to specified dmas
- * @sdev:	SOF sound device
+ * sof_probe_dma_add - attach to specified dmas
+ * @cdev:		SOF client device
  * @dma:	List of streams (dmas) to attach to
  * @num_dma:	Number of elements in @dma
  *
@@ -146,8 +150,8 @@ EXPORT_SYMBOL(sof_ipc_probe_dma_info);
  * for specifying streams which will be later used to transfer data
  * to connected probe points.
  */
-int sof_ipc_probe_dma_add(struct snd_sof_dev *sdev,
-		struct sof_probe_dma *dma, size_t num_dma)
+int sof_probe_dma_add(struct sof_client_dev *cdev,
+		      struct sof_probe_dma *dma, size_t num_dma)
 {
 	struct sof_ipc_probe_dma_add_params *msg;
 	struct sof_ipc_reply reply;
@@ -162,16 +166,16 @@ int sof_ipc_probe_dma_add(struct snd_sof_dev *sdev,
 	msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_DMA_ADD;
 	memcpy(&msg->dma[0], dma, size - sizeof(*msg));
 
-	ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
-			&reply, sizeof(reply));
+	ret = sof_client_ipc_tx_message(cdev, msg->hdr.cmd, msg, msg->hdr.size,
+					&reply, sizeof(reply));
 	kfree(msg);
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_probe_dma_add);
+EXPORT_SYMBOL(sof_probe_dma_add);
 
 /**
- * sof_ipc_probe_dma_remove - detach from specified dmas
- * @sdev:		SOF sound device
+ * sof_probe_dma_remove - detach from specified dmas
+ * @cdev:		SOF client device
  * @stream_tag:		List of stream tags to detach from
  * @num_stream_tag:	Number of elements in @stream_tag
  *
@@ -180,8 +184,8 @@ EXPORT_SYMBOL(sof_ipc_probe_dma_add);
  * match equivalent DMA_ADD. Detach only when all probes tied to
  * given stream have been disconnected.
  */
-int sof_ipc_probe_dma_remove(struct snd_sof_dev *sdev,
-		unsigned int *stream_tag, size_t num_stream_tag)
+int sof_probe_dma_remove(struct sof_client_dev *cdev,
+			 unsigned int *stream_tag, size_t num_stream_tag)
 {
 	struct sof_ipc_probe_dma_remove_params *msg;
 	struct sof_ipc_reply reply;
@@ -196,16 +200,16 @@ int sof_ipc_probe_dma_remove(struct snd_sof_dev *sdev,
 	msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_DMA_REMOVE;
 	memcpy(&msg->stream_tag[0], stream_tag, size - sizeof(*msg));
 
-	ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
-			&reply, sizeof(reply));
+	ret = sof_client_ipc_tx_message(cdev, msg->hdr.cmd, msg, msg->hdr.size,
+					&reply, sizeof(reply));
 	kfree(msg);
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_probe_dma_remove);
+EXPORT_SYMBOL(sof_probe_dma_remove);
 
 /**
- * sof_ipc_probe_points_info - retrieve list of active probe points
- * @sdev:	SOF sound device
+ * sof_probe_points_info - retrieve list of active probe points
+ * @cdev:		SOF client device
  * @desc:	Returned list of active probes
  * @num_desc:	Returned count of active probes
  *
@@ -213,17 +217,18 @@ EXPORT_SYMBOL(sof_ipc_probe_dma_remove);
  * points, valid for disconnection when given probe is no longer
  * required.
  */
-int sof_ipc_probe_points_info(struct snd_sof_dev *sdev,
-		struct sof_probe_point_desc **desc, size_t *num_desc)
+int sof_probe_points_info(struct sof_client_dev *cdev,
+			  struct sof_probe_point_desc **desc,
+			  size_t *num_desc)
 {
-	return sof_ipc_probe_info(sdev, SOF_IPC_PROBE_POINT_INFO,
+	return sof_probe_info(cdev, SOF_IPC_PROBE_POINT_INFO,
 				 (void **)desc, num_desc);
 }
-EXPORT_SYMBOL(sof_ipc_probe_points_info);
+EXPORT_SYMBOL(sof_probe_points_info);
 
 /**
- * sof_ipc_probe_points_add - connect specified probes
- * @sdev:	SOF sound device
+ * sof_probe_points_add - connect specified probes
+ * @cdev:		SOF client device
  * @desc:	List of probe points to connect
  * @num_desc:	Number of elements in @desc
  *
@@ -234,8 +239,9 @@ EXPORT_SYMBOL(sof_ipc_probe_points_info);
  * Each probe point should be removed using PROBE_POINT_REMOVE
  * request when no longer needed.
  */
-int sof_ipc_probe_points_add(struct snd_sof_dev *sdev,
-		struct sof_probe_point_desc *desc, size_t num_desc)
+int sof_probe_points_add(struct sof_client_dev *cdev,
+			 struct sof_probe_point_desc *desc,
+			 size_t num_desc)
 {
 	struct sof_ipc_probe_point_add_params *msg;
 	struct sof_ipc_reply reply;
@@ -250,24 +256,24 @@ int sof_ipc_probe_points_add(struct snd_sof_dev *sdev,
 	msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_POINT_ADD;
 	memcpy(&msg->desc[0], desc, size - sizeof(*msg));
 
-	ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
-			&reply, sizeof(reply));
+	ret = sof_client_ipc_tx_message(cdev, msg->hdr.cmd, msg, msg->hdr.size,
+					&reply, sizeof(reply));
 	kfree(msg);
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_probe_points_add);
+EXPORT_SYMBOL(sof_probe_points_add);
 
 /**
- * sof_ipc_probe_points_remove - disconnect specified probes
- * @sdev:		SOF sound device
+ * sof_probe_points_remove - disconnect specified probes
+ * @cdev:		SOF client device
  * @buffer_id:		List of probe points to disconnect
  * @num_buffer_id:	Number of elements in @desc
  *
  * Removes previously connected probes from list of active probe
  * points and frees all resources on DSP side.
  */
-int sof_ipc_probe_points_remove(struct snd_sof_dev *sdev,
-		unsigned int *buffer_id, size_t num_buffer_id)
+int sof_probe_points_remove(struct sof_client_dev *cdev,
+			    unsigned int *buffer_id, size_t num_buffer_id)
 {
 	struct sof_ipc_probe_point_remove_params *msg;
 	struct sof_ipc_reply reply;
@@ -282,9 +288,9 @@ int sof_ipc_probe_points_remove(struct snd_sof_dev *sdev,
 	msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_POINT_REMOVE;
 	memcpy(&msg->buffer_id[0], buffer_id, size - sizeof(*msg));
 
-	ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
-			&reply, sizeof(reply));
+	ret = sof_client_ipc_tx_message(cdev, msg->hdr.cmd, msg, msg->hdr.size,
+					&reply, sizeof(reply));
 	kfree(msg);
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_probe_points_remove);
+EXPORT_SYMBOL(sof_probe_points_remove);
diff --git a/sound/soc/sof/probe.h b/sound/soc/sof/probe.h
index 5e159ab239fa..42e802b7e7fc 100644
--- a/sound/soc/sof/probe.h
+++ b/sound/soc/sof/probe.h
@@ -12,8 +12,8 @@
 #define __SOF_PROBE_H
 
 #include <sound/sof/header.h>
-
-struct snd_sof_dev;
+#include <linux/debugfs.h>
+#include "sof-client.h"
 
 #define SOF_PROBE_INVALID_NODE_ID UINT_MAX
 
@@ -66,20 +66,27 @@ struct sof_ipc_probe_point_remove_params {
 	unsigned int buffer_id[];
 } __packed;
 
-int sof_ipc_probe_init(struct snd_sof_dev *sdev,
-		u32 stream_tag, size_t buffer_size);
-int sof_ipc_probe_deinit(struct snd_sof_dev *sdev);
-int sof_ipc_probe_dma_info(struct snd_sof_dev *sdev,
-		struct sof_probe_dma **dma, size_t *num_dma);
-int sof_ipc_probe_dma_add(struct snd_sof_dev *sdev,
-		struct sof_probe_dma *dma, size_t num_dma);
-int sof_ipc_probe_dma_remove(struct snd_sof_dev *sdev,
-		unsigned int *stream_tag, size_t num_stream_tag);
-int sof_ipc_probe_points_info(struct snd_sof_dev *sdev,
-		struct sof_probe_point_desc **desc, size_t *num_desc);
-int sof_ipc_probe_points_add(struct snd_sof_dev *sdev,
-		struct sof_probe_point_desc *desc, size_t num_desc);
-int sof_ipc_probe_points_remove(struct snd_sof_dev *sdev,
-		unsigned int *buffer_id, size_t num_buffer_id);
+int sof_probe_init(struct sof_client_dev *cdev, u32 stream_tag,
+		   size_t buffer_size);
+int sof_probe_deinit(struct sof_client_dev *cdev);
+int sof_probe_dma_info(struct sof_client_dev *cdev,
+		       struct sof_probe_dma **dma, size_t *num_dma);
+int sof_probe_dma_add(struct sof_client_dev *cdev,
+		      struct sof_probe_dma *dma, size_t num_dma);
+int sof_probe_dma_remove(struct sof_client_dev *cdev,
+			 unsigned int *stream_tag, size_t num_stream_tag);
+int sof_probe_points_info(struct sof_client_dev *cdev,
+			  struct sof_probe_point_desc **desc,
+			  size_t *num_desc);
+int sof_probe_points_add(struct sof_client_dev *cdev,
+			 struct sof_probe_point_desc *desc,
+			 size_t num_desc);
+int sof_probe_points_remove(struct sof_client_dev *cdev,
+			    unsigned int *buffer_id, size_t num_buffer_id);
+
+struct sof_probes_data {
+	struct dentry *dfs_root;
+	unsigned int extractor_stream_tag;
+};
 
 #endif
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index e44158410b24..121c374297fc 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -425,10 +425,6 @@ struct snd_sof_dev {
 	int ipc_timeout;
 	int boot_timeout;
 
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
-	unsigned int extractor_stream_tag;
-#endif
-
 	/* DMA for Trace */
 	struct snd_dma_buffer dmatb;
 	struct snd_dma_buffer dmatp;
diff --git a/sound/soc/sof/sof-probes-client.c b/sound/soc/sof/sof-probes-client.c
new file mode 100644
index 000000000000..73227af3d339
--- /dev/null
+++ b/sound/soc/sof/sof-probes-client.c
@@ -0,0 +1,414 @@
+// SPDX-License-Identifier: (GPL-2.0-only)
+//
+// Copyright(c) 2020 Intel Corporation. All rights reserved.
+//
+// Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
+//
+
+#include <linux/auxiliary_bus.h>
+#include <linux/completion.h>
+#include <linux/debugfs.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <sound/soc.h>
+#include "compress.h"
+#include "probe.h"
+#include "sof-client.h"
+
+#define SOF_PROBES_SUSPEND_DELAY_MS 3000
+/* only extraction supported for now */
+#define SOF_PROBES_NUM_DAI_LINKS 1
+
+/**
+ * strsplit_u32 - Split string into sequence of u32 tokens
+ * @buf:	String to split into tokens.
+ * @delim:	String containing delimiter characters.
+ * @tkns:	Returned u32 sequence pointer.
+ * @num_tkns:	Returned number of tokens obtained.
+ */
+static int
+strsplit_u32(char *buf, const char *delim, u32 **tkns, size_t *num_tkns)
+{
+	char *s;
+	u32 *data, *tmp;
+	size_t count = 0;
+	size_t cap = 32;
+	int ret = 0;
+
+	*tkns = NULL;
+	*num_tkns = 0;
+	data = kcalloc(cap, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	while ((s = strsep(&buf, delim)) != NULL) {
+		ret = kstrtouint(s, 0, data + count);
+		if (ret)
+			goto exit;
+		if (++count >= cap) {
+			cap *= 2;
+			tmp = krealloc(data, cap * sizeof(*data), GFP_KERNEL);
+			if (!tmp) {
+				ret = -ENOMEM;
+				goto exit;
+			}
+			data = tmp;
+		}
+	}
+
+	if (!count)
+		goto exit;
+	*tkns = kmemdup(data, count * sizeof(*data), GFP_KERNEL);
+	if (!(*tkns)) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+	*num_tkns = count;
+
+exit:
+	kfree(data);
+	return ret;
+}
+
+static int tokenize_input(const char __user *from, size_t count,
+			  loff_t *ppos, u32 **tkns, size_t *num_tkns)
+{
+	char *buf;
+	int ret;
+
+	buf = kmalloc(count + 1, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = simple_write_to_buffer(buf, count, ppos, from, count);
+	if (ret != count) {
+		ret = ret >= 0 ? -EIO : ret;
+		goto exit;
+	}
+
+	buf[count] = '\0';
+	ret = strsplit_u32(buf, ",", tkns, num_tkns);
+exit:
+	kfree(buf);
+	return ret;
+}
+
+static ssize_t probe_points_read(struct file *file, char __user *to,
+				 size_t count, loff_t *ppos)
+{
+	struct sof_client_dev *cdev = file->private_data;
+	struct sof_probes_data *probes_data = cdev->data;
+	struct device *dev = &cdev->auxdev.dev;
+	struct sof_probe_point_desc *desc;
+	size_t num_desc;
+	int remaining;
+	char *buf;
+	int i, ret, err;
+
+	if (probes_data->extractor_stream_tag == SOF_PROBE_INVALID_NODE_ID) {
+		dev_warn(dev, "no extractor stream running\n");
+		return -ENOENT;
+	}
+
+	buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(dev, "error: debugfs read failed to resume %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto exit;
+	}
+
+	ret = sof_probe_points_info(cdev, &desc, &num_desc);
+	if (ret < 0)
+		goto exit;
+
+	pm_runtime_mark_last_busy(dev);
+	err = pm_runtime_put_autosuspend(dev);
+	if (err < 0)
+		dev_err_ratelimited(dev, "error: debugfs read failed to idle %d\n", err);
+
+	for (i = 0; i < num_desc; i++) {
+		remaining = PAGE_SIZE - strlen(buf);
+		if (remaining > 0) {
+			ret = snprintf(buf + strlen(buf), remaining,
+				       "Id: %#010x  Purpose: %u  Node id: %#x\n",
+				       desc[i].buffer_id, desc[i].purpose, desc[i].stream_tag);
+			if (ret < 0)
+				goto free_desc;
+		} else {
+			break;
+		}
+	}
+
+	ret = simple_read_from_buffer(to, count, ppos, buf, strlen(buf));
+free_desc:
+	kfree(desc);
+exit:
+	kfree(buf);
+	return ret;
+}
+
+static ssize_t probe_points_write(struct file *file, const char __user *from,
+				  size_t count, loff_t *ppos)
+{
+	struct sof_client_dev *cdev = file->private_data;
+	struct sof_probes_data *probes_data = cdev->data;
+	struct device *dev = &cdev->auxdev.dev;
+	struct sof_probe_point_desc *desc;
+	size_t num_tkns, bytes;
+	u32 *tkns;
+	int ret, err;
+
+	if (probes_data->extractor_stream_tag == SOF_PROBE_INVALID_NODE_ID) {
+		dev_warn(dev, "no extractor stream running\n");
+		return -ENOENT;
+	}
+
+	ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
+	if (ret < 0)
+		return ret;
+	bytes = sizeof(*tkns) * num_tkns;
+	if (!num_tkns || (bytes % sizeof(*desc))) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	desc = (struct sof_probe_point_desc *)tkns;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(dev, "error: debugfs write failed to resume %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto exit;
+	}
+
+	ret = sof_probe_points_add(cdev, desc, bytes / sizeof(*desc));
+	if (!ret)
+		ret = count;
+
+	pm_runtime_mark_last_busy(dev);
+	err = pm_runtime_put_autosuspend(dev);
+	if (err < 0)
+		dev_err_ratelimited(dev, "error: debugfs write failed to idle %d\n", err);
+exit:
+	kfree(tkns);
+	return ret;
+}
+
+static const struct file_operations probe_points_fops = {
+	.open = simple_open,
+	.read = probe_points_read,
+	.write = probe_points_write,
+	.llseek = default_llseek,
+};
+
+static ssize_t
+probe_points_remove_write(struct file *file, const char __user *from,
+			  size_t count, loff_t *ppos)
+{
+	struct sof_client_dev *cdev = file->private_data;
+	struct sof_probes_data *probes_data = cdev->data;
+	struct device *dev = &cdev->auxdev.dev;
+	size_t num_tkns;
+	u32 *tkns;
+	int ret, err;
+
+	if (probes_data->extractor_stream_tag == SOF_PROBE_INVALID_NODE_ID) {
+		dev_warn(dev, "no extractor stream running\n");
+		return -ENOENT;
+	}
+
+	ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
+	if (ret < 0)
+		return ret;
+	if (!num_tkns) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err_ratelimited(dev, "error: debugfs write failed to resume %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto exit;
+	}
+
+	ret = sof_probe_points_remove(cdev, tkns, num_tkns);
+	if (!ret)
+		ret = count;
+
+	pm_runtime_mark_last_busy(dev);
+	err = pm_runtime_put_autosuspend(dev);
+	if (err < 0)
+		dev_err_ratelimited(dev, "error: debugfs write failed to idle %d\n", err);
+exit:
+	kfree(tkns);
+	return ret;
+}
+
+static const struct file_operations probe_points_remove_fops = {
+	.open = simple_open,
+	.write = probe_points_remove_write,
+	.llseek = default_llseek,
+};
+
+struct snd_soc_dai_driver sof_probes_dai_drv[] = {
+{
+	.name = "Probe Extraction CPU DAI",
+	.compress_new = snd_soc_new_compress,
+	.cops = &sof_probe_compr_ops,
+	.capture = {
+		.stream_name = "Probe Extraction",
+		.channels_min = 1,
+		.channels_max = 8,
+		.rates = SNDRV_PCM_RATE_48000,
+		.rate_min = 48000,
+		.rate_max = 48000,
+	},
+},
+};
+
+static const struct snd_soc_component_driver sof_probes_component = {
+	.name = "sof-probes-component",
+	.compress_ops = &sof_probe_compressed_ops,
+	.module_get_upon_open = 1,
+};
+
+SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY()));
+
+static struct snd_soc_card sof_probes_card = {
+	.name = "sof-probes",
+	.owner = THIS_MODULE
+};
+
+static int sof_probes_client_probe(struct auxiliary_device *auxdev,
+				   const struct auxiliary_device_id *id)
+{
+	struct sof_client_dev *cdev = auxiliary_dev_to_sof_client_dev(auxdev);
+	struct snd_soc_dai_link_component platform_component[] = {
+		{
+			.name = dev_name(&auxdev->dev),
+		}
+	};
+	struct snd_soc_card *card = &sof_probes_card;
+	struct sof_probes_data *probes_client_data;
+	struct snd_soc_dai_link_component *cpus;
+	struct snd_soc_dai_link *links;
+	int ret;
+
+	/* register probes component driver and dai */
+	ret = devm_snd_soc_register_component(&auxdev->dev, &sof_probes_component,
+					      sof_probes_dai_drv, ARRAY_SIZE(sof_probes_dai_drv));
+	if (ret < 0) {
+		dev_err(&auxdev->dev, "error: failed to register SOF probes DAI driver %d\n", ret);
+		return ret;
+	}
+
+	/* set client data */
+	probes_client_data = devm_kzalloc(&auxdev->dev, sizeof(*probes_client_data), GFP_KERNEL);
+	if (!probes_client_data)
+		return -ENOMEM;
+
+	probes_client_data->extractor_stream_tag = SOF_PROBE_INVALID_NODE_ID;
+	cdev->data = probes_client_data;
+
+	/* create probes debugfs dir under SOF debugfs root dir */
+	probes_client_data->dfs_root = debugfs_create_dir("probes",
+							  sof_client_get_debugfs_root(cdev));
+
+	/* create read-write probes_points debugfs entry */
+	debugfs_create_file("probe_points", 0644, probes_client_data->dfs_root,
+			    cdev, &probe_points_fops);
+
+	/* create read-write probe_points_remove debugfs entry */
+	debugfs_create_file("probe_points_remove", 0644, probes_client_data->dfs_root,
+			    cdev, &probe_points_remove_fops);
+
+	links = devm_kzalloc(&auxdev->dev, sizeof(*links) * SOF_PROBES_NUM_DAI_LINKS, GFP_KERNEL);
+	cpus = devm_kzalloc(&auxdev->dev, sizeof(*cpus) * SOF_PROBES_NUM_DAI_LINKS, GFP_KERNEL);
+	if (!links || !cpus)
+		return -ENOMEM;
+
+	/* extraction DAI link */
+	links[0].name = "Compress Probe Capture";
+	links[0].id = 0;
+	links[0].cpus = &cpus[0];
+	links[0].num_cpus = 1;
+	links[0].cpus->dai_name = "Probe Extraction CPU DAI";
+	links[0].codecs = dummy;
+	links[0].num_codecs = 1;
+	links[0].platforms = platform_component;
+	links[0].num_platforms = ARRAY_SIZE(platform_component);
+	links[0].nonatomic = 1;
+
+	card->num_links = SOF_PROBES_NUM_DAI_LINKS;
+	card->dai_link = links;
+	card->dev = &auxdev->dev;
+
+	/* set idle_bias_off to prevent the core from resuming the card->dev */
+	card->dapm.idle_bias_off = true;
+
+	snd_soc_card_set_drvdata(&sof_probes_card, cdev);
+
+	ret = devm_snd_soc_register_card(&auxdev->dev, card);
+	if (ret < 0) {
+		dev_err(&auxdev->dev, "error: Probes card register failed %d\n", ret);
+		return ret;
+	}
+
+	/* enable runtime PM */
+	pm_runtime_set_autosuspend_delay(&auxdev->dev, SOF_PROBES_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(&auxdev->dev);
+	pm_runtime_enable(&auxdev->dev);
+	pm_runtime_mark_last_busy(&auxdev->dev);
+	pm_runtime_idle(&auxdev->dev);
+
+	return 0;
+}
+
+static int sof_probes_client_cleanup(struct auxiliary_device *auxdev)
+{
+	struct sof_client_dev *cdev = auxiliary_dev_to_sof_client_dev(auxdev);
+	struct sof_probes_data *probes_client_data = cdev->data;
+
+	pm_runtime_disable(&auxdev->dev);
+	debugfs_remove_recursive(probes_client_data->dfs_root);
+
+	return 0;
+}
+
+static int sof_probes_client_remove(struct auxiliary_device *auxdev)
+{
+	return sof_probes_client_cleanup(auxdev);
+}
+
+static void sof_probes_client_shutdown(struct auxiliary_device *auxdev)
+{
+	sof_probes_client_cleanup(auxdev);
+}
+
+static const struct auxiliary_device_id sof_probes_auxbus_id_table[] = {
+	{ .name = "snd_sof_client.probes" },
+	{},
+};
+MODULE_DEVICE_TABLE(auxiliary, sof_probes_auxbus_id_table);
+
+/* driver name will be set based on KBUILD_MODNAME */
+static struct sof_client_drv sof_probes_test_client_drv = {
+	.auxiliary_drv = {
+		.id_table = sof_probes_auxbus_id_table,
+		.probe = sof_probes_client_probe,
+		.remove = sof_probes_client_remove,
+		.shutdown = sof_probes_client_shutdown,
+	},
+};
+
+module_sof_client_driver(sof_probes_test_client_drv);
+
+MODULE_DESCRIPTION("SOF Probes Client Driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SND_SOC_SOF_CLIENT);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 10/10] ASoC: SOF: Intel: CNL: register probes client
  2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
                   ` (8 preceding siblings ...)
  2020-11-13 16:18 ` [PATCH v4 09/10] ASoC: SOF: Add new client driver for probes support Dave Ertman
@ 2020-11-13 16:18 ` Dave Ertman
  9 siblings, 0 replies; 20+ messages in thread
From: Dave Ertman @ 2020-11-13 16:18 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, broonie, linux-rdma, jgg, dledford, netdev, davem, kuba,
	gregkh, ranjani.sridharan, pierre-louis.bossart, fred.oh, parav,
	shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel, leonro

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Register the client device for probes support on the
CNL platform. Creating this client device alleviates the
need for modifying the sound card definitions in the existing
machine drivers to add support for the new probes feature in
the FW. This will result in the creation of a separate sound
card that can be used for audio data extraction from user
specified points in the audio pipeline.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 sound/soc/sof/intel/cnl.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 20afb622c315..6d15b871dc17 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -19,6 +19,7 @@
 #include "hda.h"
 #include "hda-ipc.h"
 #include "../sof-audio.h"
+#include "../sof-client.h"
 #include "intel-client.h"
 
 static const struct snd_sof_debugfs_map cnl_dsp_debugfs[] = {
@@ -233,12 +234,26 @@ void cnl_ipc_dump(struct snd_sof_dev *sdev)
 
 static int cnl_register_clients(struct snd_sof_dev *sdev)
 {
-	return intel_register_ipc_test_clients(sdev);
+	int ret;
+
+	ret = intel_register_ipc_test_clients(sdev);
+	if (ret < 0)
+		return ret;
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+	return sof_client_dev_register(sdev, "probes", 0);
+#endif
+
+	return 0;
 }
 
 static void cnl_unregister_clients(struct snd_sof_dev *sdev)
 {
 	intel_unregister_ipc_test_clients(sdev);
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+	sof_client_dev_unregister(sdev, "probes", 0);
+#endif
 }
 
 /* cannonlake ops */
@@ -409,3 +424,4 @@ const struct sof_intel_dsp_desc jsl_chip_info = {
 };
 EXPORT_SYMBOL_NS(jsl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
 MODULE_IMPORT_NS(SND_SOC_SOF_INTEL_CLIENT);
+MODULE_IMPORT_NS(SND_SOC_SOF_CLIENT);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-13 16:18 ` [PATCH v4 01/10] Add auxiliary bus support Dave Ertman
@ 2020-11-17  5:30   ` Leon Romanovsky
  2020-11-17  7:02     ` Greg KH
  2020-11-17 13:48     ` Mark Brown
  2020-11-17 19:39   ` Martin Habets
  1 sibling, 2 replies; 20+ messages in thread
From: Leon Romanovsky @ 2020-11-17  5:30 UTC (permalink / raw)
  To: Dave Ertman, gregkh
  Cc: alsa-devel, tiwai, broonie, linux-rdma, jgg, dledford, netdev,
	davem, kuba, ranjani.sridharan, pierre-louis.bossart, fred.oh,
	parav, shiraz.saleem, dan.j.williams, kiran.patil, linux-kernel

On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> It enables drivers to create an auxiliary_device and bind an
> auxiliary_driver to it.
>
> The bus supports probe/remove shutdown and suspend/resume callbacks.
> Each auxiliary_device has a unique string based id; driver binds to
> an auxiliary_device based on this id through the bus.
>
> Co-developed-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
> Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> ---

Greg,

This horse was beaten to death, can we please progress with this patch?
Create special topic branch or ack so I'll prepare this branch.

We are in -rc4 now and we (Mellanox) can't hold our submissions anymore.
My mlx5_core probe patches [1] were too intrusive and they are ready to
be merged, Parav's patches got positive review as well [2] and will be
taken next.

We delayed and have in our internal queues the patches for VDPA, eswitch
and followup for mlx5_core probe rework, but trapped due to this AUX bus
patch.

Thanks

[1] https://lore.kernel.org/linux-rdma/20201101201542.2027568-1-leon@kernel.org/
[2] https://lore.kernel.org/linux-rdma/BY5PR12MB43229F23C101AFBCD2971534DCE20@BY5PR12MB4322.namprd12.prod.outlook.com/T/#md25b9a2feb4c60d3fef5d57da41db559af1062d8

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17  5:30   ` Leon Romanovsky
@ 2020-11-17  7:02     ` Greg KH
  2020-11-17  7:16       ` Leon Romanovsky
  2020-11-17 21:04       ` Dan Williams
  2020-11-17 13:48     ` Mark Brown
  1 sibling, 2 replies; 20+ messages in thread
From: Greg KH @ 2020-11-17  7:02 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Dave Ertman, alsa-devel, tiwai, broonie, linux-rdma, jgg,
	dledford, netdev, davem, kuba, ranjani.sridharan,
	pierre-louis.bossart, fred.oh, parav, shiraz.saleem,
	dan.j.williams, kiran.patil, linux-kernel

On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > It enables drivers to create an auxiliary_device and bind an
> > auxiliary_driver to it.
> >
> > The bus supports probe/remove shutdown and suspend/resume callbacks.
> > Each auxiliary_device has a unique string based id; driver binds to
> > an auxiliary_device based on this id through the bus.
> >
> > Co-developed-by: Kiran Patil <kiran.patil@intel.com>
> > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
> > Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> > Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> > ---
> 
> Greg,
> 
> This horse was beaten to death, can we please progress with this patch?
> Create special topic branch or ack so I'll prepare this branch.
> 
> We are in -rc4 now and we (Mellanox) can't hold our submissions anymore.
> My mlx5_core probe patches [1] were too intrusive and they are ready to
> be merged, Parav's patches got positive review as well [2] and will be
> taken next.
> 
> We delayed and have in our internal queues the patches for VDPA, eswitch
> and followup for mlx5_core probe rework, but trapped due to this AUX bus
> patch.

There are no deadlines for kernel patches here, sorry.  Give me some
time to properly review this, core kernel changes should not be rushed.

Also, if you really want to blame someone for the delay, look at the
patch submitters, not the reviewers, as they are the ones that took a
very long time with this over the lifecycle of this patchset, not me.  I
have provided many "instant" reviews of this patchset, and then months
went by between updates from them.

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17  7:02     ` Greg KH
@ 2020-11-17  7:16       ` Leon Romanovsky
  2020-11-17 21:04       ` Dan Williams
  1 sibling, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2020-11-17  7:16 UTC (permalink / raw)
  To: Greg KH
  Cc: Dave Ertman, alsa-devel, tiwai, broonie, linux-rdma, jgg,
	dledford, netdev, davem, kuba, ranjani.sridharan,
	pierre-louis.bossart, fred.oh, parav, shiraz.saleem,
	dan.j.williams, kiran.patil, linux-kernel

On Tue, Nov 17, 2020 at 08:02:53AM +0100, Greg KH wrote:
> On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > > It enables drivers to create an auxiliary_device and bind an
> > > auxiliary_driver to it.
> > >
> > > The bus supports probe/remove shutdown and suspend/resume callbacks.
> > > Each auxiliary_device has a unique string based id; driver binds to
> > > an auxiliary_device based on this id through the bus.
> > >
> > > Co-developed-by: Kiran Patil <kiran.patil@intel.com>
> > > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > > Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > > Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
> > > Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> > > Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> > > ---
> >
> > Greg,
> >
> > This horse was beaten to death, can we please progress with this patch?
> > Create special topic branch or ack so I'll prepare this branch.
> >
> > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore.
> > My mlx5_core probe patches [1] were too intrusive and they are ready to
> > be merged, Parav's patches got positive review as well [2] and will be
> > taken next.
> >
> > We delayed and have in our internal queues the patches for VDPA, eswitch
> > and followup for mlx5_core probe rework, but trapped due to this AUX bus
> > patch.
>
> There are no deadlines for kernel patches here, sorry.  Give me some
> time to properly review this, core kernel changes should not be rushed.

And here comes the difference between our views, from my POV it is not
core kernel change that must to be done perfectly from the beginning,
but change that will need to be improved/extended over time once more
users will come.

>
> Also, if you really want to blame someone for the delay, look at the
> patch submitters, not the reviewers, as they are the ones that took a
> very long time with this over the lifecycle of this patchset, not me.  I
> have provided many "instant" reviews of this patchset, and then months
> went by between updates from them.

I'm not blaming anyone and especially you. It is just unfair that I
found myself in the middle of this disaster while care enough to remind
about the series.

Thanks

>
> greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17  5:30   ` Leon Romanovsky
  2020-11-17  7:02     ` Greg KH
@ 2020-11-17 13:48     ` Mark Brown
  2020-11-17 13:57       ` Leon Romanovsky
  1 sibling, 1 reply; 20+ messages in thread
From: Mark Brown @ 2020-11-17 13:48 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Dave Ertman, gregkh, alsa-devel, tiwai, linux-rdma, jgg,
	dledford, netdev, davem, kuba, ranjani.sridharan,
	pierre-louis.bossart, fred.oh, parav, shiraz.saleem,
	dan.j.williams, kiran.patil, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:

> > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > It enables drivers to create an auxiliary_device and bind an
> > auxiliary_driver to it.

> This horse was beaten to death, can we please progress with this patch?
> Create special topic branch or ack so I'll prepare this branch.

It's been about 2 working days since the patch was last posted.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17 13:48     ` Mark Brown
@ 2020-11-17 13:57       ` Leon Romanovsky
  2020-11-17 15:48         ` Greg KH
  0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2020-11-17 13:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dave Ertman, gregkh, alsa-devel, tiwai, linux-rdma, jgg,
	dledford, netdev, davem, kuba, ranjani.sridharan,
	pierre-louis.bossart, fred.oh, parav, shiraz.saleem,
	dan.j.williams, kiran.patil, linux-kernel

On Tue, Nov 17, 2020 at 01:48:08PM +0000, Mark Brown wrote:
> On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
>
> > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > > It enables drivers to create an auxiliary_device and bind an
> > > auxiliary_driver to it.
>
> > This horse was beaten to death, can we please progress with this patch?
> > Create special topic branch or ack so I'll prepare this branch.
>
> It's been about 2 working days since the patch was last posted.

There is no code changes between v3 and v4 except docs improvements.
The v3 was posted almost 3-4 weeks ago.

Thanks

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17 13:57       ` Leon Romanovsky
@ 2020-11-17 15:48         ` Greg KH
  0 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2020-11-17 15:48 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Mark Brown, Dave Ertman, alsa-devel, tiwai, linux-rdma, jgg,
	dledford, netdev, davem, kuba, ranjani.sridharan,
	pierre-louis.bossart, fred.oh, parav, shiraz.saleem,
	dan.j.williams, kiran.patil, linux-kernel

On Tue, Nov 17, 2020 at 03:57:24PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 17, 2020 at 01:48:08PM +0000, Mark Brown wrote:
> > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> >
> > > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > > > It enables drivers to create an auxiliary_device and bind an
> > > > auxiliary_driver to it.
> >
> > > This horse was beaten to death, can we please progress with this patch?
> > > Create special topic branch or ack so I'll prepare this branch.
> >
> > It's been about 2 working days since the patch was last posted.
> 
> There is no code changes between v3 and v4 except docs improvements.
> The v3 was posted almost 3-4 weeks ago.

But everything else that came in since then needs to be reviewed first,
right?  It's a fifo queue, no jumping the line.  And, to be frank, if
people complain, that's a sure way for it to get dumped to the end of
the line, as you know.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-13 16:18 ` [PATCH v4 01/10] Add auxiliary bus support Dave Ertman
  2020-11-17  5:30   ` Leon Romanovsky
@ 2020-11-17 19:39   ` Martin Habets
  1 sibling, 0 replies; 20+ messages in thread
From: Martin Habets @ 2020-11-17 19:39 UTC (permalink / raw)
  To: Dave Ertman
  Cc: alsa-devel, tiwai, broonie, linux-rdma, jgg, dledford, netdev,
	davem, kuba, gregkh, ranjani.sridharan, pierre-louis.bossart,
	fred.oh, parav, shiraz.saleem, dan.j.williams, kiran.patil,
	linux-kernel, leonro

On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> It enables drivers to create an auxiliary_device and bind an
> auxiliary_driver to it.
> 
> The bus supports probe/remove shutdown and suspend/resume callbacks.
> Each auxiliary_device has a unique string based id; driver binds to
> an auxiliary_device based on this id through the bus.

Reviewed-by: Martin Habets <mhabets@solarflare.com>

> Co-developed-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
> Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> ---
>  Documentation/driver-api/auxiliary_bus.rst | 234 ++++++++++++++++++
>  Documentation/driver-api/index.rst         |   1 +
>  drivers/base/Kconfig                       |   3 +
>  drivers/base/Makefile                      |   1 +
>  drivers/base/auxiliary.c                   | 268 +++++++++++++++++++++
>  include/linux/auxiliary_bus.h              |  78 ++++++
>  include/linux/mod_devicetable.h            |   8 +
>  scripts/mod/devicetable-offsets.c          |   3 +
>  scripts/mod/file2alias.c                   |   8 +
>  9 files changed, 604 insertions(+)
>  create mode 100644 Documentation/driver-api/auxiliary_bus.rst
>  create mode 100644 drivers/base/auxiliary.c
>  create mode 100644 include/linux/auxiliary_bus.h
> 
> diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
> new file mode 100644
> index 000000000000..5dd7804631ef
> --- /dev/null
> +++ b/Documentation/driver-api/auxiliary_bus.rst
> @@ -0,0 +1,234 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=============
> +Auxiliary Bus
> +=============
> +
> +In some subsystems, the functionality of the core device (PCI/ACPI/other) is
> +too complex for a single device to be managed by a monolithic driver
> +(e.g. Sound Open Firmware), multiple devices might implement a common
> +intersection of functionality (e.g. NICs + RDMA), or a driver may want to
> +export an interface for another subsystem to drive (e.g. SIOV Physical Function
> +export Virtual Function management).  A split of the functinoality into child-
> +devices representing sub-domains of functionality makes it possible to
> +compartmentalize, layer, and distribute domain-specific concerns via a Linux
> +device-driver model.
> +
> +An example for this kind of requirement is the audio subsystem where a single
> +IP is handling multiple entities such as HDMI, Soundwire, local devices such as
> +mics/speakers etc. The split for the core's functionality can be arbitrary or
> +be defined by the DSP firmware topology and include hooks for test/debug. This
> +allows for the audio core device to be minimal and focused on hardware-specific
> +control and communication.
> +
> +Each auxiliary_device represents a part of its parent functionality. The
> +generic behavior can be extended and specialized as needed by encapsulating an
> +auxiliary_device within other domain-specific structures and the use of .ops
> +callbacks. Devices on the auxiliary bus do not share any structures and the use
> +of a communication channel with the parent is domain-specific.
> +
> +Note that ops are intended as a way to augment instance behavior within a class
> +of auxiliary devices, it is not the mechanism for exporting common
> +infrastructure from the parent. Consider EXPORT_SYMBOL_NS() to convey
> +infrastructure from the parent module to the auxiliary module(s).
> +
> +
> +When Should the Auxiliary Bus Be Used
> +=====================================
> +
> +The auxiliary bus is to be used when a driver and one or more kernel modules,
> +who share a common header file with the driver, need a mechanism to connect and
> +provide access to a shared object allocated by the auxiliary_device's
> +registering driver.  The registering driver for the auxiliary_device(s) and the
> +kernel module(s) registering auxiliary_drivers can be from the same subsystem,
> +or from multiple subsystems.
> +
> +The emphasis here is on a common generic interface that keeps subsystem
> +customization out of the bus infrastructure.
> +
> +One example is a PCI network device that is RDMA-capable and exports a child
> +device to be driven by an auxiliary_driver in the RDMA subsystem.  The PCI
> +driver allocates and registers an auxiliary_device for each physical
> +function on the NIC.  The RDMA driver registers an auxiliary_driver that claims
> +each of these auxiliary_devices.  This conveys data/ops published by the parent
> +PCI device/driver to the RDMA auxiliary_driver.
> +
> +Another use case is for the PCI device to be split out into multiple sub
> +functions.  For each sub function an auxiliary_device is created.  A PCI sub
> +function driver binds to such devices that creates its own one or more class
> +devices.  A PCI sub function auxiliary device is likely to be contained in a
> +struct with additional attributes such as user defined sub function number and
> +optional attributes such as resources and a link to the parent device.  These
> +attributes could be used by systemd/udev; and hence should be initialized
> +before a driver binds to an auxiliary_device.
> +
> +A key requirement for utilizing the auxiliary bus is that there is no
> +dependency on a physical bus, device, register accesses or regmap support.
> +These individual devices split from the core cannot live on the platform bus as
> +they are not physical devices that are controlled by DT/ACPI.  The same
> +argument applies for not using MFD in this scenario as MFD relies on individual
> +function devices being physical devices.
> +
> +Auxiliary Device
> +================
> +
> +An auxiliary_device represents a part of its parent device's functionality. It
> +is given a name that, combined with the registering drivers KBUILD_MODNAME,
> +creates a match_name that is used for driver binding, and an id that combined
> +with the match_name provide a unique name to register with the bus subsystem.
> +
> +Registering an auxiliary_device is a two-step process.  First call
> +auxiliary_device_init(), which checks several aspects of the auxiliary_device
> +struct and performs a device_initialize().  After this step completes, any
> +error state must have a call to auxiliary_device_uninit() in its resolution path.
> +The second step in registering an auxiliary_device is to perform a call to
> +auxiliary_device_add(), which sets the name of the device and add the device to
> +the bus.
> +
> +Unregistering an auxiliary_device is also a two-step process to mirror the
> +register process.  First call auxiliary_device_delete(), then call
> +auxiliary_device_uninit().
> +
> +.. code-block:: c
> +
> +	struct auxiliary_device {
> +		struct device dev;
> +                const char *name;
> +		u32 id;
> +	};
> +
> +If two auxiliary_devices both with a match_name "mod.foo" are registered onto
> +the bus, they must have unique id values (e.g. "x" and "y") so that the
> +registered devices names are "mod.foo.x" and "mod.foo.y".  If match_name + id
> +are not unique, then the device_add fails and generates an error message.
> +
> +The auxiliary_device.dev.type.release or auxiliary_device.dev.release must be
> +populated with a non-NULL pointer to successfully register the auxiliary_device.
> +
> +The auxiliary_device.dev.parent must also be populated.
> +
> +Auxiliary Device Memory Model and Lifespan
> +------------------------------------------
> +
> +The registering driver is the entity that allocates memory for the
> +auxiliary_device and register it on the auxiliary bus.  It is important to note
> +that, as opposed to the platform bus, the registering driver is wholly
> +responsible for the management for the memory used for the driver object.
> +
> +A parent object, defined in the shared header file, contains the
> +auxiliary_device.  It also contains a pointer to the shared object(s), which
> +also is defined in the shared header.  Both the parent object and the shared
> +object(s) are allocated by the registering driver.  This layout allows the
> +auxiliary_driver's registering module to perform a container_of() call to go
> +from the pointer to the auxiliary_device, that is passed during the call to the
> +auxiliary_driver's probe function, up to the parent object, and then have
> +access to the shared object(s).
> +
> +The memory for the auxiliary_device is freed only in its release() callback
> +flow as defined by its registering driver.
> +
> +The memory for the shared object(s) must have a lifespan equal to, or greater
> +than, the lifespan of the memory for the auxiliary_device.  The auxiliary_driver
> +should only consider that this shared object is valid as long as the
> +auxiliary_device is still registered on the auxiliary bus.  It is up to the
> +registering driver to manage (e.g. free or keep available) the memory for the
> +shared object beyond the life of the auxiliary_device.
> +
> +The registering driver must unregister all auxiliary devices before its own
> +driver.remove() is completed.
> +
> +Auxiliary Drivers
> +=================
> +
> +Auxiliary drivers follow the standard driver model convention, where
> +discovery/enumeration is handled by the core, and drivers
> +provide probe() and remove() methods. They support power management
> +and shutdown notifications using the standard conventions.
> +
> +.. code-block:: c
> +
> +	struct auxiliary_driver {
> +		int (*probe)(struct auxiliary_device *,
> +                             const struct auxiliary_device_id *id);
> +		int (*remove)(struct auxiliary_device *);
> +		void (*shutdown)(struct auxiliary_device *);
> +		int (*suspend)(struct auxiliary_device *, pm_message_t);
> +		int (*resume)(struct auxiliary_device *);
> +		struct device_driver driver;
> +		const struct auxiliary_device_id *id_table;
> +	};
> +
> +Auxiliary drivers register themselves with the bus by calling
> +auxiliary_driver_register(). The id_table contains the match_names of auxiliary
> +devices that a driver can bind with.
> +
> +Example Usage
> +=============
> +
> +Auxiliary devices are created and registered by a subsystem-level core device
> +that needs to break up its functionality into smaller fragments. One way to
> +extend the scope of an auxiliary_device is to encapsulate it within a domain-
> +pecific structure defined by the parent device. This structure contains the
> +auxiliary_device and any associated shared data/callbacks needed to establish
> +the connection with the parent.
> +
> +An example is:
> +
> +.. code-block:: c
> +
> +        struct foo {
> +		struct auxiliary_device auxdev;
> +		void (*connect)(struct auxiliary_device *auxdev);
> +		void (*disconnect)(struct auxiliary_device *auxdev);
> +		void *data;
> +        };
> +
> +The parent device then registers the auxiliary_device by calling
> +auxiliary_device_init(), and then auxiliary_device_add(), with the pointer to
> +the auxdev member of the above structure. The parent provides a name for the
> +auxiliary_device that, combined with the parent's KBUILD_MODNAME, creates a
> +match_name that is be used for matching and binding with a driver.
> +
> +Whenever an auxiliary_driver is registered, based on the match_name, the
> +auxiliary_driver's probe() is invoked for the matching devices.  The
> +auxiliary_driver can also be encapsulated inside custom drivers that make the
> +core device's functionality extensible by adding additional domain-specific ops
> +as follows:
> +
> +.. code-block:: c
> +
> +	struct my_ops {
> +		void (*send)(struct auxiliary_device *auxdev);
> +		void (*receive)(struct auxiliary_device *auxdev);
> +	};
> +
> +
> +	struct my_driver {
> +		struct auxiliary_driver auxiliary_drv;
> +		const struct my_ops ops;
> +	};
> +
> +An example of this type of usage is:
> +
> +.. code-block:: c
> +
> +	const struct auxiliary_device_id my_auxiliary_id_table[] = {
> +		{ .name = "foo_mod.foo_dev" },
> +		{ },
> +	};
> +
> +	const struct my_ops my_custom_ops = {
> +		.send = my_tx,
> +		.receive = my_rx,
> +	};
> +
> +	const struct my_driver my_drv = {
> +		.auxiliary_drv = {
> +			.name = "myauxiliarydrv",
> +			.id_table = my_auxiliary_id_table,
> +			.probe = my_probe,
> +			.remove = my_remove,
> +			.shutdown = my_shutdown,
> +		},
> +		.ops = my_custom_ops,
> +	};
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> index f357f3eb400c..86759a74b7f1 100644
> --- a/Documentation/driver-api/index.rst
> +++ b/Documentation/driver-api/index.rst
> @@ -72,6 +72,7 @@ available subsections can be seen below.
>     thermal/index
>     fpga/index
>     acpi/index
> +   auxiliary_bus
>     backlight/lp855x-driver.rst
>     connector
>     console
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 8d7001712062..040be48ce046 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -1,6 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0
>  menu "Generic Driver Options"
>  
> +config AUXILIARY_BUS
> +	bool
> +
>  config UEVENT_HELPER
>  	bool "Support for uevent helper"
>  	help
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 41369fc7004f..5e7bf9669a81 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -7,6 +7,7 @@ obj-y			:= component.o core.o bus.o dd.o syscore.o \
>  			   attribute_container.o transport_class.o \
>  			   topology.o container.o property.o cacheinfo.o \
>  			   swnode.o
> +obj-$(CONFIG_AUXILIARY_BUS) += auxiliary.o
>  obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
>  obj-y			+= power/
>  obj-$(CONFIG_ISA_BUS_API)	+= isa.o
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> new file mode 100644
> index 000000000000..ef2af417438b
> --- /dev/null
> +++ b/drivers/base/auxiliary.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2019-2020 Intel Corporation
> + *
> + * Please see Documentation/driver-api/auxiliary_bus.rst for more information.
> + */
> +
> +#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__
> +
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/string.h>
> +#include <linux/auxiliary_bus.h>
> +
> +static const struct auxiliary_device_id *auxiliary_match_id(const struct auxiliary_device_id *id,
> +							    const struct auxiliary_device *auxdev)
> +{
> +	for (; id->name[0]; id++) {
> +		const char *p = strrchr(dev_name(&auxdev->dev), '.');
> +		int match_size;
> +
> +		if (!p)
> +			continue;
> +		match_size = p - dev_name(&auxdev->dev);
> +
> +		/* use dev_name(&auxdev->dev) prefix before last '.' char to match to */
> +		if (strlen(id->name) == match_size &&
> +		    !strncmp(dev_name(&auxdev->dev), id->name, match_size))
> +			return id;
> +	}
> +	return NULL;
> +}
> +
> +static int auxiliary_match(struct device *dev, struct device_driver *drv)
> +{
> +	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> +	struct auxiliary_driver *auxdrv = to_auxiliary_drv(drv);
> +
> +	return !!auxiliary_match_id(auxdrv->id_table, auxdev);
> +}
> +
> +static int auxiliary_uevent(struct device *dev, struct kobj_uevent_env *env)
> +{
> +	const char *name, *p;
> +
> +	name = dev_name(dev);
> +	p = strrchr(name, '.');
> +
> +	return add_uevent_var(env, "MODALIAS=%s%.*s", AUXILIARY_MODULE_PREFIX, (int)(p - name),
> +			      name);
> +}
> +
> +static const struct dev_pm_ops auxiliary_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(pm_generic_runtime_suspend, pm_generic_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
> +};
> +
> +static int auxiliary_bus_probe(struct device *dev)
> +{
> +	struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
> +	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> +	int ret;
> +
> +	ret = dev_pm_domain_attach(dev, true);
> +	if (ret) {
> +		dev_warn(dev, "Failed to attach to PM Domain : %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = auxdrv->probe(auxdev, auxiliary_match_id(auxdrv->id_table, auxdev));
> +	if (ret)
> +		dev_pm_domain_detach(dev, true);
> +
> +	return ret;
> +}
> +
> +static int auxiliary_bus_remove(struct device *dev)
> +{
> +	struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
> +	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> +	int ret = 0;
> +
> +	if (auxdrv->remove)
> +		ret = auxdrv->remove(auxdev);
> +	dev_pm_domain_detach(dev, true);
> +
> +	return ret;
> +}
> +
> +static void auxiliary_bus_shutdown(struct device *dev)
> +{
> +	struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
> +	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> +
> +	if (auxdrv->shutdown)
> +		auxdrv->shutdown(auxdev);
> +}
> +
> +static struct bus_type auxiliary_bus_type = {
> +	.name = "auxiliary",
> +	.probe = auxiliary_bus_probe,
> +	.remove = auxiliary_bus_remove,
> +	.shutdown = auxiliary_bus_shutdown,
> +	.match = auxiliary_match,
> +	.uevent = auxiliary_uevent,
> +	.pm = &auxiliary_dev_pm_ops,
> +};
> +
> +/**
> + * auxiliary_device_init - check auxiliary_device and initialize
> + * @auxdev: auxiliary device struct
> + *
> + * This is the first step in the two-step process to register an auxiliary_device.
> + *
> + * When this function returns an error code, then the device_initialize will *not* have
> + * been performed, and the caller will be responsible to free any memory allocated for the
> + * auxiliary_device in the error path directly.
> + *
> + * It returns 0 on success.  On success, the device_initialize has been performed.  After this
> + * point any error unwinding will need to include a call to auxiliary_device_uninit().
> + * In this post-initialize error scenario, a call to the device's .release callback will be
> + * triggered, and all memory clean-up is expected to be handled there.
> + */
> +int auxiliary_device_init(struct auxiliary_device *auxdev)
> +{
> +	struct device *dev = &auxdev->dev;
> +
> +	if (!dev->parent) {
> +		pr_err("auxiliary_device has a NULL dev->parent\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!auxdev->name) {
> +		pr_err("auxiliary_device has a NULL name\n");
> +		return -EINVAL;
> +	}
> +
> +	dev->bus = &auxiliary_bus_type;
> +	device_initialize(&auxdev->dev);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_device_init);
> +
> +/**
> + * __auxiliary_device_add - add an auxiliary bus device
> + * @auxdev: auxiliary bus device to add to the bus
> + * @modname: name of the parent device's driver module
> + *
> + * This is the second step in the two-step process to register an auxiliary_device.
> + *
> + * This function must be called after a successful call to auxiliary_device_init(), which
> + * will perform the device_initialize.  This means that if this returns an error code, then a
> + * call to auxiliary_device_uninit() must be performed so that the .release callback will
> + * be triggered to free the memory associated with the auxiliary_device.
> + *
> + * The expectation is that users will call the "auxiliary_device_add" macro so that the caller's
> + * KBUILD_MODNAME is automatically inserted for the modname parameter.  Only if a user requires
> + * a custom name would this version be called directly.
> + */
> +int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname)
> +{
> +	struct device *dev = &auxdev->dev;
> +	int ret;
> +
> +	if (!modname) {
> +		pr_err("auxiliary device modname is NULL\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = dev_set_name(dev, "%s.%s.%d", modname, auxdev->name, auxdev->id);
> +	if (ret) {
> +		pr_err("auxiliary device dev_set_name failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = device_add(dev);
> +	if (ret)
> +		dev_err(dev, "adding auxiliary device failed!: %d\n", ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(__auxiliary_device_add);
> +
> +/**
> + * auxiliary_find_device - auxiliary device iterator for locating a particular device.
> + * @start: Device to begin with
> + * @data: Data to pass to match function
> + * @match: Callback function to check device
> + *
> + * This function returns a reference to a device that is 'found'
> + * for later use, as determined by the @match callback.
> + *
> + * The callback should return 0 if the device doesn't match and non-zero
> + * if it does.  If the callback returns non-zero, this function will
> + * return to the caller and not iterate over any more devices.
> + */
> +struct auxiliary_device *
> +auxiliary_find_device(struct device *start, const void *data,
> +		      int (*match)(struct device *dev, const void *data))
> +{
> +	struct device *dev;
> +
> +	dev = bus_find_device(&auxiliary_bus_type, start, data, match);
> +	if (!dev)
> +		return NULL;
> +
> +	return to_auxiliary_dev(dev);
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_find_device);
> +
> +/**
> + * __auxiliary_driver_register - register a driver for auxiliary bus devices
> + * @auxdrv: auxiliary_driver structure
> + * @owner: owning module/driver
> + * @modname: KBUILD_MODNAME for parent driver
> + */
> +int __auxiliary_driver_register(struct auxiliary_driver *auxdrv, struct module *owner,
> +				const char *modname)
> +{
> +	if (WARN_ON(!auxdrv->probe) || WARN_ON(!auxdrv->id_table))
> +		return -EINVAL;
> +
> +	if (auxdrv->name)
> +		auxdrv->driver.name = kasprintf(GFP_KERNEL, "%s.%s", modname, auxdrv->name);
> +	else
> +		auxdrv->driver.name = kasprintf(GFP_KERNEL, "%s", modname);
> +	if (!auxdrv->driver.name)
> +		return -ENOMEM;
> +
> +	auxdrv->driver.owner = owner;
> +	auxdrv->driver.bus = &auxiliary_bus_type;
> +	auxdrv->driver.mod_name = modname;
> +
> +	return driver_register(&auxdrv->driver);
> +}
> +EXPORT_SYMBOL_GPL(__auxiliary_driver_register);
> +
> +/**
> + * auxiliary_driver_unregister - unregister a driver
> + * @auxdrv: auxiliary_driver structure
> + */
> +void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
> +{
> +	driver_unregister(&auxdrv->driver);
> +	kfree(auxdrv->driver.name);
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
> +
> +static int __init auxiliary_bus_init(void)
> +{
> +	return bus_register(&auxiliary_bus_type);
> +}
> +
> +static void __exit auxiliary_bus_exit(void)
> +{
> +	bus_unregister(&auxiliary_bus_type);
> +}
> +
> +module_init(auxiliary_bus_init);
> +module_exit(auxiliary_bus_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Auxiliary Bus");
> +MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>");
> +MODULE_AUTHOR("Kiran Patil <kiran.patil@intel.com>");
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> new file mode 100644
> index 000000000000..282fbf7bf9af
> --- /dev/null
> +++ b/include/linux/auxiliary_bus.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2019-2020 Intel Corporation
> + *
> + * Please see Documentation/driver-api/auxiliary_bus.rst for more information.
> + */
> +
> +#ifndef _AUXILIARY_BUS_H_
> +#define _AUXILIARY_BUS_H_
> +
> +#include <linux/device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/slab.h>
> +
> +struct auxiliary_device {
> +	struct device dev;
> +	const char *name;
> +	u32 id;
> +};
> +
> +struct auxiliary_driver {
> +	int (*probe)(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id);
> +	int (*remove)(struct auxiliary_device *auxdev);
> +	void (*shutdown)(struct auxiliary_device *auxdev);
> +	int (*suspend)(struct auxiliary_device *auxdev, pm_message_t state);
> +	int (*resume)(struct auxiliary_device *auxdev);
> +	const char *name;
> +	struct device_driver driver;
> +	const struct auxiliary_device_id *id_table;
> +};
> +
> +static inline struct auxiliary_device *to_auxiliary_dev(struct device *dev)
> +{
> +	return container_of(dev, struct auxiliary_device, dev);
> +}
> +
> +static inline struct auxiliary_driver *to_auxiliary_drv(struct device_driver *drv)
> +{
> +	return container_of(drv, struct auxiliary_driver, driver);
> +}
> +
> +int auxiliary_device_init(struct auxiliary_device *auxdev);
> +int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname);
> +#define auxiliary_device_add(auxdev) __auxiliary_device_add(auxdev, KBUILD_MODNAME)
> +
> +static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
> +{
> +	put_device(&auxdev->dev);
> +}
> +
> +static inline void auxiliary_device_delete(struct auxiliary_device *auxdev)
> +{
> +	device_del(&auxdev->dev);
> +}
> +
> +int __auxiliary_driver_register(struct auxiliary_driver *auxdrv, struct module *owner,
> +				const char *modname);
> +#define auxiliary_driver_register(auxdrv) \
> +	__auxiliary_driver_register(auxdrv, THIS_MODULE, KBUILD_MODNAME)
> +
> +void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv);
> +
> +/**
> + * module_auxiliary_driver() - Helper macro for registering an auxiliary driver
> + * @__auxiliary_driver: auxiliary driver struct
> + *
> + * Helper macro for auxiliary drivers which do not do anything special in
> + * module init/exit. This eliminates a lot of boilerplate. Each module may only
> + * use this macro once, and calling it replaces module_init() and module_exit()
> + */
> +#define module_auxiliary_driver(__auxiliary_driver) \
> +	module_driver(__auxiliary_driver, auxiliary_driver_register, auxiliary_driver_unregister)
> +
> +struct auxiliary_device *
> +auxiliary_find_device(struct device *start, const void *data,
> +		      int (*match)(struct device *dev, const void *data));
> +
> +#endif /* _AUXILIARY_BUS_H_ */
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 5b08a473cdba..c425290b21e2 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -838,4 +838,12 @@ struct mhi_device_id {
>  	kernel_ulong_t driver_data;
>  };
>  
> +#define AUXILIARY_NAME_SIZE 32
> +#define AUXILIARY_MODULE_PREFIX "auxiliary:"
> +
> +struct auxiliary_device_id {
> +	char name[AUXILIARY_NAME_SIZE];
> +	kernel_ulong_t driver_data;
> +};
> +
>  #endif /* LINUX_MOD_DEVICETABLE_H */
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index 27007c18e754..e377f52dbfa3 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -243,5 +243,8 @@ int main(void)
>  	DEVID(mhi_device_id);
>  	DEVID_FIELD(mhi_device_id, chan);
>  
> +	DEVID(auxiliary_device_id);
> +	DEVID_FIELD(auxiliary_device_id, name);
> +
>  	return 0;
>  }
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 2417dd1dee33..fb4827027536 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1364,6 +1364,13 @@ static int do_mhi_entry(const char *filename, void *symval, char *alias)
>  {
>  	DEF_FIELD_ADDR(symval, mhi_device_id, chan);
>  	sprintf(alias, MHI_DEVICE_MODALIAS_FMT, *chan);
> +	return 1;
> +}
> +
> +static int do_auxiliary_entry(const char *filename, void *symval, char *alias)
> +{
> +	DEF_FIELD_ADDR(symval, auxiliary_device_id, name);
> +	sprintf(alias, AUXILIARY_MODULE_PREFIX "%s", *name);
>  
>  	return 1;
>  }
> @@ -1442,6 +1449,7 @@ static const struct devtable devtable[] = {
>  	{"tee", SIZE_tee_client_device_id, do_tee_entry},
>  	{"wmi", SIZE_wmi_device_id, do_wmi_entry},
>  	{"mhi", SIZE_mhi_device_id, do_mhi_entry},
> +	{"auxiliary", SIZE_auxiliary_device_id, do_auxiliary_entry},
>  };
>  
>  /* Create MODULE_ALIAS() statements.
> -- 
> 2.26.2

-- 
Martin Habets <mhabets@solarflare.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17  7:02     ` Greg KH
  2020-11-17  7:16       ` Leon Romanovsky
@ 2020-11-17 21:04       ` Dan Williams
  2020-11-18  6:19         ` Greg KH
  1 sibling, 1 reply; 20+ messages in thread
From: Dan Williams @ 2020-11-17 21:04 UTC (permalink / raw)
  To: Greg KH
  Cc: Leon Romanovsky, Dave Ertman, alsa-devel, Takashi Iwai,
	Mark Brown, linux-rdma, Jason Gunthorpe, Doug Ledford, Netdev,
	David Miller, Jakub Kicinski, Ranjani Sridharan,
	Pierre-Louis Bossart, Fred Oh, Parav Pandit, Saleem, Shiraz,
	Patil, Kiran, Linux Kernel Mailing List

On Mon, Nov 16, 2020 at 11:02 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > > It enables drivers to create an auxiliary_device and bind an
> > > auxiliary_driver to it.
> > >
> > > The bus supports probe/remove shutdown and suspend/resume callbacks.
> > > Each auxiliary_device has a unique string based id; driver binds to
> > > an auxiliary_device based on this id through the bus.
> > >
> > > Co-developed-by: Kiran Patil <kiran.patil@intel.com>
> > > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > > Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > > Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
> > > Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> > > Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> > > ---
> >
> > Greg,
> >
> > This horse was beaten to death, can we please progress with this patch?
> > Create special topic branch or ack so I'll prepare this branch.
> >
> > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore.
> > My mlx5_core probe patches [1] were too intrusive and they are ready to
> > be merged, Parav's patches got positive review as well [2] and will be
> > taken next.
> >
> > We delayed and have in our internal queues the patches for VDPA, eswitch
> > and followup for mlx5_core probe rework, but trapped due to this AUX bus
> > patch.
>
> There are no deadlines for kernel patches here, sorry.  Give me some
> time to properly review this, core kernel changes should not be rushed.
>
> Also, if you really want to blame someone for the delay, look at the
> patch submitters not the reviewers, as they are the ones that took a
> very long time with this over the lifecycle of this patchset, not me.  I
> have provided many "instant" reviews of this patchset, and then months
> went by between updates from them.

Please stop this finger pointing. It was already noted that the team,
out of abundance of caution / deference to the process, decided not to
push the patches while I was out on family leave. It's cruel to hold
that against them, and if anyone is to blame it's me for not
clarifying it was ok to proceed while I was out.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 01/10] Add auxiliary bus support
  2020-11-17 21:04       ` Dan Williams
@ 2020-11-18  6:19         ` Greg KH
  0 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2020-11-18  6:19 UTC (permalink / raw)
  To: Dan Williams
  Cc: Leon Romanovsky, Dave Ertman, alsa-devel, Takashi Iwai,
	Mark Brown, linux-rdma, Jason Gunthorpe, Doug Ledford, Netdev,
	David Miller, Jakub Kicinski, Ranjani Sridharan,
	Pierre-Louis Bossart, Fred Oh, Parav Pandit, Saleem, Shiraz,
	Patil, Kiran, Linux Kernel Mailing List

On Tue, Nov 17, 2020 at 01:04:56PM -0800, Dan Williams wrote:
> On Mon, Nov 16, 2020 at 11:02 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote:
> > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote:
> > > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver.
> > > > It enables drivers to create an auxiliary_device and bind an
> > > > auxiliary_driver to it.
> > > >
> > > > The bus supports probe/remove shutdown and suspend/resume callbacks.
> > > > Each auxiliary_device has a unique string based id; driver binds to
> > > > an auxiliary_device based on this id through the bus.
> > > >
> > > > Co-developed-by: Kiran Patil <kiran.patil@intel.com>
> > > > Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> > > > Co-developed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > > > Co-developed-by: Fred Oh <fred.oh@linux.intel.com>
> > > > Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> > > > Co-developed-by: Leon Romanovsky <leonro@nvidia.com>
> > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > > Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > > > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > > > Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> > > > ---
> > >
> > > Greg,
> > >
> > > This horse was beaten to death, can we please progress with this patch?
> > > Create special topic branch or ack so I'll prepare this branch.
> > >
> > > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore.
> > > My mlx5_core probe patches [1] were too intrusive and they are ready to
> > > be merged, Parav's patches got positive review as well [2] and will be
> > > taken next.
> > >
> > > We delayed and have in our internal queues the patches for VDPA, eswitch
> > > and followup for mlx5_core probe rework, but trapped due to this AUX bus
> > > patch.
> >
> > There are no deadlines for kernel patches here, sorry.  Give me some
> > time to properly review this, core kernel changes should not be rushed.
> >
> > Also, if you really want to blame someone for the delay, look at the
> > patch submitters not the reviewers, as they are the ones that took a
> > very long time with this over the lifecycle of this patchset, not me.  I
> > have provided many "instant" reviews of this patchset, and then months
> > went by between updates from them.
> 
> Please stop this finger pointing. It was already noted that the team,
> out of abundance of caution / deference to the process, decided not to
> push the patches while I was out on family leave. It's cruel to hold
> that against them, and if anyone is to blame it's me for not
> clarifying it was ok to proceed while I was out.

I'm not blaming anyone, I'm just getting pissed when people are
insisting that I do "quick reviews" for this patchset, which has been
happening by different people since the very beginning of this whole
feature, so I am trying to explain where others should be pointing their
frustration at instead of me if they really want to do such a thing
(hint, they shouldn't, but I wasn't explicit about that, sorry).

Combine this with the long delays between my reviews and a new patchset
submission, and on my end it's an extremely frustrating situation, which
frankly, makes me want to review this thing even less and less as I know
it's not going to be a fun or easy time when I do so.

Everyone needs to remember that there are no deadlines here, and the
people involved all have other things to work on at the same time, and
that there are a lot of different subsystems and moving parts all
involved.  So someone is going to get grumpy about it, and right now, it
seems to be me.  I know I need to review this, and complaining that I
haven't done so within 3 days of sending an updated patch set is not
helping anyone.

I'm going to try to carve out some time this week to review this
properly.  Hopefully there's no other major security "scares" popping up
like there was the past few weeks to divert me from this...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-11-18  6:20 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 16:18 [PATCH v4 00/10] Auxiliary bus implementation and SOF multi-client support Dave Ertman
2020-11-13 16:18 ` [PATCH v4 01/10] Add auxiliary bus support Dave Ertman
2020-11-17  5:30   ` Leon Romanovsky
2020-11-17  7:02     ` Greg KH
2020-11-17  7:16       ` Leon Romanovsky
2020-11-17 21:04       ` Dan Williams
2020-11-18  6:19         ` Greg KH
2020-11-17 13:48     ` Mark Brown
2020-11-17 13:57       ` Leon Romanovsky
2020-11-17 15:48         ` Greg KH
2020-11-17 19:39   ` Martin Habets
2020-11-13 16:18 ` [PATCH v4 02/10] ASoC: SOF: Introduce descriptors for SOF client Dave Ertman
2020-11-13 16:18 ` [PATCH v4 03/10] ASoC: SOF: Create client driver for IPC test Dave Ertman
2020-11-13 16:18 ` [PATCH v4 04/10] ASoC: SOF: ops: Add ops for client registration Dave Ertman
2020-11-13 16:18 ` [PATCH v4 05/10] ASoC: SOF: Intel: Define " Dave Ertman
2020-11-13 16:18 ` [PATCH v4 06/10] ASoC: SOF: Intel: Remove IPC flood test support in SOF core Dave Ertman
2020-11-13 16:18 ` [PATCH v4 07/10] ASoC: SOF: sof-client: Add client APIs to access probes ops Dave Ertman
2020-11-13 16:18 ` [PATCH v4 08/10] ASoC: SOF: compress: move and export sof_probe_compr_ops Dave Ertman
2020-11-13 16:18 ` [PATCH v4 09/10] ASoC: SOF: Add new client driver for probes support Dave Ertman
2020-11-13 16:18 ` [PATCH v4 10/10] ASoC: SOF: Intel: CNL: register probes client Dave Ertman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).