On 07-12-23, 16:29, Pierre-Louis Bossart wrote:
Add definitions and helpers for the BPT/BRA protocol. Peripheral drivers (aka ASoC codec drivers) can use this API to send bulk data such as firmware or tables.
The API is only available when no other audio streams have been allocated, and only one BTP/BRA stream is allowed per link. To avoid the addition of yet another lock, the refcount tests are handled in the stream master_runtime alloc/free routines where the bus_lock is already held. Another benefit of this approach is that the same bus_lock is used to handle runtime and port linked lists, which reduces the potential for misaligned configurations.
In addition to exclusion with audio streams, BPT transfers have a lot of overhead, specifically registers writes are needed to enable transport in DP0. In addition, most DMAs don't handle too well very small data sets.
This patch suggests a minimum bound of 64 bytes, for smaller transfers codec drivers should rely on the regular read/write commands in Column0.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
drivers/soundwire/bus.c | 77 +++++++++++++++++++++++++++++++++++ drivers/soundwire/bus.h | 18 ++++++++ drivers/soundwire/stream.c | 30 ++++++++++++++ include/linux/soundwire/sdw.h | 76 ++++++++++++++++++++++++++++++++++ 4 files changed, 201 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index f3fec15c3112..e5758d2ed88f 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -2015,3 +2015,80 @@ void sdw_clear_slave_status(struct sdw_bus *bus, u32 request) } } EXPORT_SYMBOL(sdw_clear_slave_status);
+int sdw_bpt_close_stream(struct sdw_bus *bus,
struct sdw_slave *slave,enum sdw_bpt_type mode,struct sdw_bpt_msg *msg)+{
- int ret;
- ret = bus->ops->bpt_close_stream(bus, slave, mode, msg);
- if (ret < 0)
dev_err(bus->dev, "BPT stream close, err %d\n", ret);- return ret;
+} +EXPORT_SYMBOL(sdw_bpt_close_stream);
+int sdw_bpt_open_stream(struct sdw_bus *bus,
struct sdw_slave *slave,enum sdw_bpt_type mode,struct sdw_bpt_msg *msg)+{
- int ret;
- /* only Bulk Register Access (BRA) is supported for now */
- if (mode != SDW_BRA)
return -EINVAL;- if (msg->len < SDW_BPT_MSG_MIN_BYTES) {
dev_err(bus->dev, "BPT message length %d, min supported %d\n",msg->len, SDW_BPT_MSG_MIN_BYTES);return -EINVAL;- }
- if (msg->len % SDW_BPT_MSG_BYTE_ALIGNMENT) {
dev_err(bus->dev, "BPT message length %d is not a multiple of %d bytes\n",msg->len, SDW_BPT_MSG_BYTE_ALIGNMENT);return -EINVAL;- }
Is this a protocol requirement?
- /* check device is enumerated */
- if (slave->dev_num == SDW_ENUM_DEV_NUM ||
slave->dev_num > SDW_MAX_DEVICES)return -ENODEV;- /* make sure all callbacks are defined */
- if (!bus->ops->bpt_open_stream ||
!bus->ops->bpt_close_stream ||!bus->ops->bpt_send_async ||!bus->ops->bpt_wait)return -ENOTSUPP;
should this not be checked at probe time, if device declares the support
- ret = bus->ops->bpt_open_stream(bus, slave, mode, msg);
- if (ret < 0)
dev_err(bus->dev, "BPT stream open, err %d\n", ret);- return ret;
+} +EXPORT_SYMBOL(sdw_bpt_open_stream);
can we open multiple times (i dont see a check preventing that), how do we close..?
Re-iterating my comment on documentation patch, can we do with a async api and wait api, that makes symantics a lot simpler, right..?
+int sdw_bpt_send_async(struct sdw_bus *bus,
struct sdw_slave *slave,struct sdw_bpt_msg *msg)+{
- if (msg->len > SDW_BPT_MSG_MAX_BYTES)
return -EINVAL;- return bus->ops->bpt_send_async(bus, slave, msg);
+} +EXPORT_SYMBOL(sdw_bpt_send_async);
Can we call this multiple times after open, it is unclear to me. Can you please add kernel-doc comments about the APIs here as well
struct sdw_master_ops { int (*read_prop)(struct sdw_bus *bus); @@ -869,6 +913,20 @@ struct sdw_master_ops { void (*new_peripheral_assigned)(struct sdw_bus *bus, struct sdw_slave *slave, int dev_num);
- int (*bpt_open_stream)(struct sdw_bus *bus,
struct sdw_slave *slave,enum sdw_bpt_type mode,struct sdw_bpt_msg *msg);- int (*bpt_close_stream)(struct sdw_bus *bus,
struct sdw_slave *slave,enum sdw_bpt_type mode,struct sdw_bpt_msg *msg);- int (*bpt_send_async)(struct sdw_bus *bus,
struct sdw_slave *slave,struct sdw_bpt_msg *msg);- int (*bpt_wait)(struct sdw_bus *bus,
struct sdw_slave *slave,struct sdw_bpt_msg *msg);
do we need both bus and slave, that was a mistake in orignal design IMO. We should fix that for bpt_ apis