diff mbox

[v4,04/15] soundwire: Add MIPI DisCo property helpers

Message ID 1512122177-2889-5-git-send-email-vinod.koul@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vinod Koul Dec. 1, 2017, 9:56 a.m. UTC
MIPI Discovery And Configuration (DisCo) Specification for SoundWire
specifies properties to be implemented for SoundWire Masters and
Slaves. The DisCo spec doesn't mandate these properties. However,
SDW bus cannot work without knowing these values.

The helper functions read the Master and Slave properties.
Implementers of Master or Slave drivers can use any of the below
three mechanisms:
   a) Use these APIs here as .read_prop() callback for Master
      and Slave
   b) Implement own methods and set those as .read_prop(), but invoke
      APIs in this file for generic read and override the values with
      platform specific data
   c) Implement ones own methods which do not use anything provided
      here

Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/soundwire/Makefile     |   2 +-
 drivers/soundwire/bus.c        |   8 +
 drivers/soundwire/bus_type.c   |  23 ++-
 drivers/soundwire/mipi_disco.c | 374 +++++++++++++++++++++++++++++++++++++++++
 include/linux/soundwire/sdw.h  | 274 ++++++++++++++++++++++++++++++
 5 files changed, 679 insertions(+), 2 deletions(-)
 create mode 100644 drivers/soundwire/mipi_disco.c

Comments

Pierre-Louis Bossart Dec. 1, 2017, 10:49 p.m. UTC | #1
On 12/1/17 3:56 AM, Vinod Koul wrote:
> MIPI Discovery And Configuration (DisCo) Specification for SoundWire
> specifies properties to be implemented for SoundWire Masters and
> Slaves. The DisCo spec doesn't mandate these properties. However,
> SDW bus cannot work without knowing these values.
> 
> The helper functions read the Master and Slave properties.
> Implementers of Master or Slave drivers can use any of the below
> three mechanisms:
>     a) Use these APIs here as .read_prop() callback for Master
>        and Slave
>     b) Implement own methods and set those as .read_prop(), but invoke
>        APIs in this file for generic read and override the values with
>        platform specific data
>     c) Implement ones own methods which do not use anything provided
>        here
> 
> Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>   drivers/soundwire/Makefile     |   2 +-
>   drivers/soundwire/bus.c        |   8 +
>   drivers/soundwire/bus_type.c   |  23 ++-
>   drivers/soundwire/mipi_disco.c | 374 +++++++++++++++++++++++++++++++++++++++++
>   include/linux/soundwire/sdw.h  | 274 ++++++++++++++++++++++++++++++
>   5 files changed, 679 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/soundwire/mipi_disco.c
> 
> diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
> index c875e434f8b3..bcde0d26524c 100644
> --- a/drivers/soundwire/Makefile
> +++ b/drivers/soundwire/Makefile
> @@ -3,5 +3,5 @@
>   #
>   
>   #Bus Objs
> -soundwire-bus-objs := bus_type.o bus.o slave.o
> +soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o
>   obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index 0f89b2f36938..507ae85ad58e 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -25,6 +25,14 @@ int sdw_add_bus_master(struct sdw_bus *bus)
>   	mutex_init(&bus->bus_lock);
>   	INIT_LIST_HEAD(&bus->slaves);
>   
> +	if (bus->ops->read_prop) {
> +		ret = bus->ops->read_prop(bus);
> +		if (ret < 0) {
> +			dev_err(bus->dev, "Bus read properties failed:%d", ret);
> +			return ret;
> +		}
> +	}
> +
>   	/*
>   	 * Device numbers in SoundWire are 0 thru 15 with 0 being
>   	 * Enumeration device number and 15 broadcast device number. So
> diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
> index 8d8dcc68e9a8..d5f3a70c06b0 100644
> --- a/drivers/soundwire/bus_type.c
> +++ b/drivers/soundwire/bus_type.c
> @@ -77,6 +77,8 @@ static int sdw_drv_probe(struct device *dev)
>   	if (!id)
>   		return -ENODEV;
>   
> +	slave->ops = drv->ops;
> +
>   	/*
>   	 * attach to power domain but don't turn on (last arg)
>   	 */
> @@ -89,7 +91,26 @@ static int sdw_drv_probe(struct device *dev)
>   		}
>   	}
>   
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	/* device is probed so let's read the properties now */
> +	if (slave->ops && slave->ops->read_prop)
> +		slave->ops->read_prop(slave);
> +
> +	/*
> +	 * Check for valid clk_stop_timeout, use DisCo worst case value of
> +	 * 300ms
> +	 *
> +	 * TODO: check the timeouts and driver removal case
> +	 */
> +	if (slave->prop.clk_stop_timeout == 0)
> +		slave->prop.clk_stop_timeout = 300;
> +
> +	slave->bus->clk_stop_timeout = max_t(u32, slave->bus->clk_stop_timeout,
> +					slave->prop.clk_stop_timeout);
> +
> +	return 0;
>   }
>   
>   static int sdw_drv_remove(struct device *dev)
> diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
> new file mode 100644
> index 000000000000..b8870d4a25ba
> --- /dev/null
> +++ b/drivers/soundwire/mipi_disco.c
> @@ -0,0 +1,374 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +// Copyright(c) 2015-17 Intel Corporation.
> +
> +/*
> + * MIPI Discovery And Configuration (DisCo) Specification for SoundWire
> + * specifies properties to be implemented for SoundWire Masters and Slaves.
> + * The DisCo spec doesn't mandate these properties. However, SDW bus cannot
> + * work without knowing these values.
> + *
> + * The helper functions read the Master and Slave properties. Implementers
> + * of Master or Slave drivers can use any of the below three mechanisms:
> + *    a) Use these APIs here as .read_prop() callback for Master and Slave
> + *    b) Implement own methods and set those as .read_prop(), but invoke
> + *    APIs in this file for generic read and override the values with
> + *    platform specific data
> + *    c) Implement ones own methods which do not use anything provided
> + *    here
> + */
> +
> +#include <linux/device.h>
> +#include <linux/property.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/soundwire/sdw.h>
> +#include "bus.h"
> +
> +/**
> + * sdw_master_read_prop() - Read Master properties
> + * @bus: SDW bus instance
> + */
> +int sdw_master_read_prop(struct sdw_bus *bus)
> +{
> +	struct sdw_master_prop *prop = &bus->prop;
> +	struct fwnode_handle *link;
> +	unsigned int count = 0;
> +	char name[32];
> +	int nval, i;
> +
> +	device_property_read_u32(bus->dev,
> +			"mipi-sdw-sw-interface-revision", &prop->revision);
> +	device_property_read_u32(bus->dev, "mipi-sdw-master-count", &count);
> +
> +	/* Find link handle */
> +	snprintf(name, sizeof(name),
> +			"mipi-sdw-link-%d-subproperties", bus->link_id);

if you follow the DisCo spec, this property is at the controller level, 
isn't there a confusion between controller/master here, and consequently 
are we reading the same things multiple times or using the wrong bus 
parameter?

If I look at intel_probe(), there is a clear reference to a link_id, and 
then you set the pointer to this read_prop which reads the number of 
links, which looks like the wrong order. You can't assign a link ID 
before knowing how many links there are - or you may be unable to detect 
issues.

> +
> +	link = device_get_named_child_node(bus->dev, name);
> +	if (!link) {
> +		dev_err(bus->dev, "Link node %s not found\n", name);
> +		return -EIO;
> +	}
> +
> +	if (fwnode_property_read_bool(link,
> +			"mipi-sdw-clock-stop-mode0-supported") == true)
> +		prop->clk_stop_mode = SDW_CLK_STOP_MODE0;
> +
> +	if (fwnode_property_read_bool(link,
> +			"mipi-sdw-clock-stop-mode1-supported") == true)
> +		prop->clk_stop_mode |= SDW_CLK_STOP_MODE1;
> +
> +	fwnode_property_read_u32(link,
> +			"mipi-sdw-max-clock-frequency", &prop->max_freq);
> +
> +	nval = fwnode_property_read_u32_array(link,
> +			"mipi-sdw-clock-frequencies-supported", NULL, 0);
> +	if (nval > 0)
> +		prop->num_freq = nval;
> +
> +	if (prop->num_freq) {
> +		prop->freq = devm_kcalloc(bus->dev, nval,
> +					sizeof(*prop->freq), GFP_KERNEL);
> +		if (!prop->freq)
> +			return -ENOMEM;
> +
> +		fwnode_property_read_u32_array(link,
> +				"mipi-sdw-clock-frequencies-supported",
> +				prop->freq, nval);
> +
> +		/*
> +		 * Check the frequencies supported. If FW doesn't provide max
> +		 * freq, then populate here by checking values.
> +		 */
> +		if (!prop->max_freq) {
> +			prop->max_freq = prop->freq[0];
> +			for (i = 1; i < prop->num_freq; i++) {
> +				if (prop->freq[i] > prop->max_freq)
> +					prop->max_freq = prop->freq[i];
> +			}
> +		}
> +	}
> +
> +	nval = fwnode_property_read_u32_array(link,
> +			"mipi-sdw-supported-clock-gears", NULL, 0);
> +	if (nval > 0)
> +		prop->num_clk_gears = nval;
> +
> +	if (prop->num_clk_gears) {
> +		prop->clk_gears = devm_kcalloc(bus->dev, nval,
> +				sizeof(*prop->clk_gears), GFP_KERNEL);
> +		if (!prop->clk_gears)
> +			return -ENOMEM;
> +
> +		fwnode_property_read_u32_array(link,
> +				"mipi-sdw-supported-clock-gears",
> +				prop->clk_gears, nval);
> +	}
> +
> +	fwnode_property_read_u32(link, "mipi-sdw-default-frame-rate",
> +				&prop->default_frame_rate);
> +	fwnode_property_read_u32(link, "mipi-sdw-default-frame-row-size",
> +				&prop->default_row);
> +	fwnode_property_read_u32(link, "mipi-sdw-default-frame-col-size",

This is fine, just wondering if we should warnings if the values make no 
sense, e.g. the DisCo spec states in Note1 page 15 that the values are 
interrelated.

> +				&prop->default_col);
> +	prop->dynamic_frame =  fwnode_property_read_bool(link,
> +				"mipi-sdw-dynamic-frame-shape");
> +	fwnode_property_read_u32(link, "mipi-sdw-command-error-threshold",
> +				&prop->err_threshold);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(sdw_master_read_prop);
> +
> +static int sdw_slave_read_dpn(struct sdw_slave *slave,
> +		struct sdw_dpn_prop *dpn, int count, int ports, char *type)
> +{
> +	struct fwnode_handle *node;
> +	u32 bit, i = 0, nval;
> +	unsigned long addr;
> +	char name[40];
> +
> +	addr = ports;
> +	/* valid ports are 1 to 14 so apply mask */
> +	addr &= GENMASK(14, 1);
> +
> +	for_each_set_bit(bit, &addr, 32) {
> +		snprintf(name, sizeof(name),
> +			"mipi-sdw-dp-%d-%s-subproperties", bit, type);
> +
> +		dpn[i].num = bit;
> +
> +		node = device_get_named_child_node(&slave->dev, name);
> +		if (!node) {
> +			dev_err(&slave->dev, "%s dpN not found\n", name);
> +			return -EIO;
> +		}
> +
> +		fwnode_property_read_u32(node, "mipi-sdw-port-max-wordlength",
> +						&dpn[i].max_word);
> +		fwnode_property_read_u32(node, "mipi-sdw-port-min-wordlength",
> +						&dpn[i].min_word);
> +
> +		nval = fwnode_property_read_u32_array(node,
> +				"mipi-sdw-port-wordlength-configs", NULL, 0);
> +		if (nval > 0)
> +			dpn[i].num_words = nval;
> +
> +		if (dpn[i].num_words) {
> +			dpn[i].words = devm_kcalloc(&slave->dev, nval,
> +				sizeof(*dpn[i].words), GFP_KERNEL);
> +			if (!dpn[i].words)
> +				return -ENOMEM;
> +
> +			fwnode_property_read_u32_array(node,
> +					"mipi-sdw-port-wordlength-configs",
> +					&dpn[i].num_words, nval);
> +		}
> +
> +		fwnode_property_read_u32(node, "mipi-sdw-data-port-type",
> +				&dpn[i].type);
> +		fwnode_property_read_u32(node,
> +				"mipi-sdw-max-grouping-supported",
> +				&dpn[i].max_grouping);
> +		dpn[i].simple_ch_prep_sm = fwnode_property_read_bool(node,
> +				"mipi-sdw-simplified-channelprepare-sm");
> +		fwnode_property_read_u32(node,
> +				"mipi-sdw-port-channelprepare-timeout",
> +				&dpn[i].ch_prep_timeout);
> +		fwnode_property_read_u32(node,
> +				"mipi-sdw-imp-def-dpn-interrupts-supported",
> +				&dpn[i].device_interrupts);
> +		fwnode_property_read_u32(node, "mipi-sdw-min-channel-number",
> +				&dpn[i].min_ch);
> +		fwnode_property_read_u32(node, "mipi-sdw-max-channel-number",
> +				&dpn[i].max_ch);
> +
> +		nval = fwnode_property_read_u32_array(node,
> +				"mipi-sdw-channel-number-list", NULL, 0);
> +		if (nval > 0)
> +			dpn[i].num_ch = nval;
> +
> +		if (dpn[i].num_ch) {
> +			dpn[i].ch = devm_kcalloc(&slave->dev, nval,
> +					sizeof(*dpn[i].ch), GFP_KERNEL);
> +			if (!dpn[i].ch)
> +				return -ENOMEM;
> +
> +			fwnode_property_read_u32_array(node,
> +					"mipi-sdw-channel-number-list",
> +					dpn[i].ch, nval);
> +		}
> +
> +		nval = fwnode_property_read_u32_array(node,
> +				"mipi-sdw-channel-combination-list", NULL, 0);
> +		if (nval > 0)
> +			dpn[i].num_ch_combinations = nval;
> +
> +		if (dpn[i].num_ch_combinations) {
> +			dpn[i].ch_combinations = devm_kcalloc(&slave->dev,
> +					nval, sizeof(*dpn[i].ch_combinations),
> +					GFP_KERNEL);
> +			if (!dpn[i].ch_combinations)
> +				return -ENOMEM;
> +
> +			fwnode_property_read_u32_array(node,
> +					"mipi-sdw-channel-combination-list",
> +					dpn[i].ch_combinations, nval);
> +		}
> +
> +		fwnode_property_read_u32(node,
> +				"mipi-sdw-modes-supported", &dpn[i].modes);
> +		fwnode_property_read_u32(node, "mipi-sdw-max-async-buffer",
> +				&dpn[i].max_async_buffer);
> +		dpn[i].block_pack_mode = fwnode_property_read_bool(node,
> +				"mipi-sdw-block-packing-mode");
> +
> +		fwnode_property_read_u32(node, "mipi-sdw-port-encoding-type",
> +				&dpn[i].port_encoding);
> +
> +		/* TODO: Read audio mode */
> +
> +		i++;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * sdw_slave_read_prop() - Read Slave properties
> + * @slave: SDW Slave
> + */
> +int sdw_slave_read_prop(struct sdw_slave *slave)
> +{
> +	struct sdw_slave_prop *prop = &slave->prop;
> +	struct device *dev = &slave->dev;
> +	struct fwnode_handle *port;
> +	int num_of_ports, nval, i;
> +
> +	device_property_read_u32(dev, "mipi-sdw-sw-interface-revision",
> +				&prop->mipi_revision);
> +
> +	prop->wake_capable = device_property_read_bool(dev,
> +				"mipi-sdw-wake-up-unavailable");
> +	prop->wake_capable = !prop->wake_capable;
> +
> +	prop->test_mode_capable = device_property_read_bool(dev,
> +				"mipi-sdw-test-mode-supported");
> +
> +	prop->clk_stop_mode1 = false;
> +	if (device_property_read_bool(dev,
> +				"mipi-sdw-clock-stop-mode1-supported"))
> +		prop->clk_stop_mode1 = true;
> +
> +	prop->simple_clk_stop_capable = device_property_read_bool(dev,
> +			"mipi-sdw-simplified-clockstopprepare-sm-supported");
> +
> +	device_property_read_u32(dev, "mipi-sdw-clockstopprepare-timeout",
> +			&prop->clk_stop_timeout);
> +	device_property_read_u32(dev, "mipi-sdw-slave-channelprepare-timeout",
> +			&prop->ch_prep_timeout);
> +	device_property_read_u32(dev,
> +			"mipi-sdw-clockstopprepare-hard-reset-behavior",
> +			&prop->reset_behave);
> +
> +	prop->high_PHY_capable = device_property_read_bool(dev,
> +			"mipi-sdw-highPHY-capable");
> +	prop->paging_support = device_property_read_bool(dev,
> +			"mipi-sdw-paging-support");
> +	prop->bank_delay_support = device_property_read_bool(dev,
> +			"mipi-sdw-bank-delay-support");
> +	device_property_read_u32(dev,
> +			"mipi-sdw-port15-read-behavior", &prop->p15_behave);
> +	device_property_read_u32(dev, "mipi-sdw-master-count",
> +				&prop->master_count);
> +
> +	device_property_read_u32(dev, "mipi-sdw-source-port-list",
> +				&prop->source_ports);
> +	device_property_read_u32(dev, "mipi-sdw-sink-port-list",
> +				&prop->sink_ports);
> +
> +	/* Read dp0 properties */
> +	port = device_get_named_child_node(dev, "mipi-sdw-dp-0-subproperties");
> +	if (!port) {
> +		dev_err(dev, "DP0 node not found!!\n");
> +		return -EIO;
> +	}
> +
> +	prop->dp0_prop = devm_kzalloc(&slave->dev,
> +			sizeof(*prop->dp0_prop), GFP_KERNEL);
> +	if (!prop->dp0_prop)
> +		return -ENOMEM;
> +
> +	fwnode_property_read_u32(port, "mipi-sdw-port-max-wordlength",
> +			&prop->dp0_prop->max_word);
> +	fwnode_property_read_u32(port, "mipi-sdw-port-min-wordlength",
> +			&prop->dp0_prop->min_word);
> +	nval = fwnode_property_read_u32_array(port,
> +			"mipi-sdw-port-wordlength-configs", NULL, 0);
> +	if (nval > 0)
> +		prop->dp0_prop->num_words = nval;
> +
> +	if (prop->dp0_prop->num_words) {
> +		prop->dp0_prop->words = devm_kcalloc(&slave->dev, nval,
> +				sizeof(*prop->dp0_prop->words), GFP_KERNEL);
> +		if (!prop->dp0_prop->words)
> +			return -ENOMEM;
> +
> +		fwnode_property_read_u32_array(port,
> +				"mipi-sdw-port-wordlength-configs",
> +				prop->dp0_prop->words, nval);
> +	}
> +
> +	prop->dp0_prop->flow_controlled = fwnode_property_read_bool(port,
> +			"mipi-sdw-bra-flow-controlled");
> +
> +	prop->dp0_prop->simple_ch_prep_sm = fwnode_property_read_bool(port,
> +			"mipi-sdw-simplified-channel-prepare-sm");
> +
> +	prop->dp0_prop->device_interrupts = fwnode_property_read_bool(port,
> +			"mipi-sdw-imp-def-dp0-interrupts-supported");
> +
> +	/*
> +	 * Based on each DPn port, get source and sink dpn properties.
> +	 * Also, some ports can operate as both source or sink.
> +	 */
> +
> +	/* Allocate memory for set bits in port lists */
> +	nval = hweight32(prop->source_ports);
> +	num_of_ports += nval;

this and...

> +	prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> +				sizeof(*prop->src_dpn_prop), GFP_KERNEL);
> +	if (!prop->src_dpn_prop)
> +		return -ENOMEM;
> +
> +	/* Read dpn properties for source port(s) */
> +	sdw_slave_read_dpn(slave, prop->src_dpn_prop, nval,
> +			prop->source_ports, "source");
> +
> +	nval = hweight32(prop->sink_ports);
> +	num_of_ports += nval;

... this is no longer needed since...

> +	prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> +				sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
> +	if (!prop->sink_dpn_prop)
> +		return -ENOMEM;
> +
> +	/* Read dpn properties for sink port(s) */
> +	sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
> +			prop->sink_ports, "sink");
> +
> +	/* some ports are bidirectional so check total ports by ORing */
> +	nval = prop->source_ports | prop->sink_ports;
> +	num_of_ports = hweight32(nval) + 1; /* add 1 for DP0 */

... you reassign the value here. That was one earlier feedback from me 
but you left the variable incrementation in the code.

> +
> +	/* Allocate port_ready based on num_of_ports */
> +	slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
> +				sizeof(*slave->port_ready), GFP_KERNEL);
> +	if (!slave->port_ready)
> +		return -ENOMEM;
> +
> +	/* Initialize completion */
> +	for (i = 0; i < num_of_ports; i++ > +		init_completion(&slave->port_ready[i]);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(sdw_slave_read_prop);
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index 7e9579941c66..ddfae18f4306 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -32,6 +32,252 @@ enum sdw_slave_status {
>   };
>   
>   /*
> + * SDW properties, defined in MIPI DisCo spec v1.0
> + */
> +enum sdw_clk_stop_reset_behave {
> +	SDW_CLK_STOP_KEEP_STATUS = 1,
> +};
> +
> +/**
> + * enum sdw_p15_behave - Slave Port 15 behaviour when the Master attempts a
> + * read
> + * @SDW_P15_READ_IGNORED: Read is ignored
> + * @SDW_P15_CMD_OK: Command is ok
> + */
> +enum sdw_p15_behave {
> +	SDW_P15_READ_IGNORED = 0,
> +	SDW_P15_CMD_OK = 1,
> +};
> +
> +/**
> + * enum sdw_dpn_type - Data port types
> + * @SDW_DPN_FULL: Full Data Port is supported
> + * @SDW_DPN_SIMPLE: Simplified Data Port as defined in spec.
> + * DPN_SampleCtrl2, DPN_OffsetCtrl2, DPN_HCtrl and DPN_BlockCtrl3
> + * are not implemented.
> + * @SDW_DPN_REDUCED: Reduced Data Port as defined in spec.
> + * DPN_SampleCtrl2, DPN_HCtrl are not implemented.
> + */
> +enum sdw_dpn_type {
> +	SDW_DPN_FULL = 0,
> +	SDW_DPN_SIMPLE = 1,
> +	SDW_DPN_REDUCED = 2,
> +};
> +
> +/**
> + * enum sdw_clk_stop_mode - Clock Stop modes
> + * @SDW_CLK_STOP_MODE0: Slave can continue operation seamlessly on clock
> + * restart
> + * @SDW_CLK_STOP_MODE1: Slave may have entered a deeper power-saving mode,
> + * not capable of continuing operation seamlessly when the clock restarts
> + */
> +enum sdw_clk_stop_mode {
> +	SDW_CLK_STOP_MODE0 = 1,
> +	SDW_CLK_STOP_MODE1 = 2,

why not 0 and 1?

> +};
> +
> +/**
> + * struct sdw_dp0_prop - DP0 properties
> + * @max_word: Maximum number of bits in a Payload Channel Sample, 1 to 64
> + * (inclusive)
> + * @min_word: Minimum number of bits in a Payload Channel Sample, 1 to 64
> + * (inclusive)
> + * @num_words: number of wordlengths supported
> + * @words: wordlengths supported
> + * @flow_controlled: Slave implementation results in an OK_NotReady
> + * response
> + * @simple_ch_prep_sm: If channel prepare sequence is required
> + * @device_interrupts: If implementation-defined interrupts are supported
> + *
> + * The wordlengths are specified by Spec as max, min AND number of
> + * discrete values, implementation can define based on the wordlengths they
> + * support
> + */
> +struct sdw_dp0_prop {
> +	u32 max_word;
> +	u32 min_word;
> +	u32 num_words;
> +	u32 *words;
> +	bool flow_controlled;
> +	bool simple_ch_prep_sm;
> +	bool device_interrupts;
> +};
> +
> +/**
> + * struct sdw_dpn_audio_mode - Audio mode properties for DPn
> + * @bus_min_freq: Minimum bus frequency, in Hz
> + * @bus_max_freq: Maximum bus frequency, in Hz
> + * @bus_num_freq: Number of discrete frequencies supported
> + * @bus_freq: Discrete bus frequencies, in Hz
> + * @min_freq: Minimum sampling frequency, in Hz
> + * @max_freq: Maximum sampling bus frequency, in Hz
> + * @num_freq: Number of discrete sampling frequency supported
> + * @freq: Discrete sampling frequencies, in Hz
> + * @prep_ch_behave: Specifies the dependencies between Channel Prepare
> + * sequence and bus clock configuration
> + * If 0, Channel Prepare can happen at any Bus clock rate
> + * If 1, Channel Prepare sequence shall happen only after Bus clock is
> + * changed to a frequency supported by this mode or compatible modes
> + * described by the next field
> + * @glitchless: Bitmap describing possible glitchless transitions from this
> + * Audio Mode to other Audio Modes
> + */
> +struct sdw_dpn_audio_mode {
> +	u32 bus_min_freq;
> +	u32 bus_max_freq;
> +	u32 bus_num_freq;
> +	u32 *bus_freq;
> +	u32 max_freq;
> +	u32 min_freq;
> +	u32 num_freq;
> +	u32 *freq;
> +	u32 prep_ch_behave;
> +	u32 glitchless;
> +};
> +
> +/**
> + * struct sdw_dpn_prop - Data Port DPn properties
> + * @num: port number
> + * @max_word: Maximum number of bits in a Payload Channel Sample, 1 to 64
> + * (inclusive)
> + * @min_word: Minimum number of bits in a Payload Channel Sample, 1 to 64
> + * (inclusive)
> + * @num_words: Number of discrete supported wordlengths
> + * @words: Discrete supported wordlength
> + * @type: Data port type. Full, Simplified or Reduced
> + * @max_grouping: Maximum number of samples that can be grouped together for
> + * a full data port
> + * @simple_ch_prep_sm: If the port supports simplified channel prepare state
> + * machine
> + * @ch_prep_timeout: Port-specific timeout value, in milliseconds
> + * @device_interrupts: If set, each bit corresponds to support for
> + * implementation-defined interrupts
> + * @max_ch: Maximum channels supported
> + * @min_ch: Minimum channels supported
> + * @num_ch: Number of discrete channels supported
> + * @ch: Discrete channels supported
> + * @num_ch_combinations: Number of channel combinations supported
> + * @ch_combinations: Channel combinations supported
> + * @modes: SDW mode supported
> + * @max_async_buffer: Number of samples that this port can buffer in
> + * asynchronous modes
> + * @block_pack_mode: Type of block port mode supported
> + * @port_encoding: Payload Channel Sample encoding schemes supported
> + * @audio_modes: Audio modes supported
> + */
> +struct sdw_dpn_prop {
> +	u32 num;
> +	u32 max_word;
> +	u32 min_word;
> +	u32 num_words;
> +	u32 *words;
> +	enum sdw_dpn_type type;
> +	u32 max_grouping;
> +	bool simple_ch_prep_sm;
> +	u32 ch_prep_timeout;
> +	u32 device_interrupts;
> +	u32 max_ch;
> +	u32 min_ch;
> +	u32 num_ch;
> +	u32 *ch;
> +	u32 num_ch_combinations;
> +	u32 *ch_combinations;
> +	u32 modes;
> +	u32 max_async_buffer;
> +	bool block_pack_mode;
> +	u32 port_encoding;
> +	struct sdw_dpn_audio_mode *audio_modes;
> +};
> +
> +/**
> + * struct sdw_slave_prop - SoundWire Slave properties
> + * @mipi_revision: Spec version of the implementation
> + * @wake_capable: Wake-up events are supported
> + * @test_mode_capable: If test mode is supported
> + * @clk_stop_mode1: Clock-Stop Mode 1 is supported
> + * @simple_clk_stop_capable: Simple clock mode is supported
> + * @clk_stop_timeout: Worst-case latency of the Clock Stop Prepare State
> + * Machine transitions, in milliseconds
> + * @ch_prep_timeout: Worst-case latency of the Channel Prepare State Machine
> + * transitions, in milliseconds
> + * @reset_behave: Slave keeps the status of the SlaveStopClockPrepare
> + * state machine (P=1 SCSP_SM) after exit from clock-stop mode1
> + * @high_PHY_capable: Slave is HighPHY capable
> + * @paging_support: Slave implements paging registers SCP_AddrPage1 and
> + * SCP_AddrPage2
> + * @bank_delay_support: Slave implements bank delay/bridge support registers
> + * SCP_BankDelay and SCP_NextFrame
> + * @p15_behave: Slave behavior when the Master attempts a read to the Port15
> + * alias
> + * @lane_control_support: Slave supports lane control
> + * @master_count: Number of Masters present on this Slave
> + * @source_ports: Bitmap identifying source ports
> + * @sink_ports: Bitmap identifying sink ports
> + * @dp0_prop: Data Port 0 properties
> + * @src_dpn_prop: Source Data Port N properties
> + * @sink_dpn_prop: Sink Data Port N properties
> + */
> +struct sdw_slave_prop {
> +	u32 mipi_revision;
> +	bool wake_capable;
> +	bool test_mode_capable;
> +	bool clk_stop_mode1;
> +	bool simple_clk_stop_capable;
> +	u32 clk_stop_timeout;
> +	u32 ch_prep_timeout;
> +	enum sdw_clk_stop_reset_behave reset_behave;
> +	bool high_PHY_capable;
> +	bool paging_support;
> +	bool bank_delay_support;
> +	enum sdw_p15_behave p15_behave;
> +	bool lane_control_support;
> +	u32 master_count;
> +	u32 source_ports;
> +	u32 sink_ports;
> +	struct sdw_dp0_prop *dp0_prop;
> +	struct sdw_dpn_prop *src_dpn_prop;
> +	struct sdw_dpn_prop *sink_dpn_prop;
> +};
> +
> +/**
> + * struct sdw_master_prop - Master properties
> + * @revision: MIPI spec version of the implementation
> + * @master_count: Number of masters
> + * @clk_stop_mode: Bitmap for Clock Stop modes supported
> + * @max_freq: Maximum Bus clock frequency, in Hz
> + * @num_clk_gears: Number of clock gears supported
> + * @clk_gears: Clock gears supported
> + * @num_freq: Number of clock frequencies supported, in Hz
> + * @freq: Clock frequencies supported, in Hz
> + * @default_frame_rate: Controller default Frame rate, in Hz
> + * @default_row: Number of rows
> + * @default_col: Number of columns
> + * @dynamic_frame: Dynamic frame supported
> + * @err_threshold: Number of times that software may retry sending a single
> + * command
> + * @dpn_prop: Data Port N properties
> + */
> +struct sdw_master_prop {
> +	u32 revision;
> +	u32 master_count;
> +	enum sdw_clk_stop_mode clk_stop_mode;
> +	u32 max_freq;
> +	u32 num_clk_gears;
> +	u32 *clk_gears;
> +	u32 num_freq;
> +	u32 *freq;
> +	u32 default_frame_rate;
> +	u32 default_row;
> +	u32 default_col;
> +	bool dynamic_frame;
> +	u32 err_threshold;
> +	struct sdw_dpn_prop *dpn_prop;
> +};
> +
> +int sdw_master_read_prop(struct sdw_bus *bus);
> +int sdw_slave_read_prop(struct sdw_slave *slave);
> +
> +/*
>    * SDW Slave Structures and APIs
>    */
>   
> @@ -55,12 +301,23 @@ struct sdw_slave_id {
>   };
>   
>   /**
> + * struct sdw_slave_ops - Slave driver callback ops
> + * @read_prop: Read Slave properties
> + */
> +struct sdw_slave_ops {
> +	int (*read_prop)(struct sdw_slave *sdw);
> +};
> +
> +/**
>    * struct sdw_slave - SoundWire Slave
>    * @id: MIPI device ID
>    * @dev: Linux device
>    * @status: Status reported by the Slave
>    * @bus: Bus handle
> + * @ops: Slave callback ops
> + * @prop: Slave properties
>    * @node: node for bus list
> + * @port_ready: Port ready completion flag for each Slave port
>    * @dev_num: Device Number assigned by Bus
>    */
>   struct sdw_slave {
> @@ -68,7 +325,10 @@ struct sdw_slave {
>   	struct device dev;
>   	enum sdw_slave_status status;
>   	struct sdw_bus *bus;
> +	const struct sdw_slave_ops *ops;
> +	struct sdw_slave_prop prop;
>   	struct list_head node;
> +	struct completion *port_ready;
>   	u16 dev_num;
>   };
>   
> @@ -100,6 +360,14 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
>    */
>   
>   /**
> + * struct sdw_master_ops - Master driver ops
> + * @read_prop: Read Master properties
> + */
> +struct sdw_master_ops {
> +	int (*read_prop)(struct sdw_bus *bus);
> +};
> +
> +/**
>    * struct sdw_bus - SoundWire bus
>    * @dev: Master linux device
>    * @link_id: Link id number, can be 0 to N, unique for each Master
> @@ -107,6 +375,9 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
>    * @assigned: Bitmap for Slave device numbers.
>    * Bit set implies used number, bit clear implies unused number.
>    * @bus_lock: bus lock
> + * @ops: Master callback ops
> + * @prop: Master properties
> + * @clk_stop_timeout: Clock stop timeout computed
>    */
>   struct sdw_bus {
>   	struct device *dev;
> @@ -114,6 +385,9 @@ struct sdw_bus {
>   	struct list_head slaves;
>   	DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
>   	struct mutex bus_lock;
> +	const struct sdw_master_ops *ops;
> +	struct sdw_master_prop prop;
> +	unsigned int clk_stop_timeout;
>   };
>   
>   int sdw_add_bus_master(struct sdw_bus *bus);
>
Vinod Koul Dec. 3, 2017, 4:52 p.m. UTC | #2
On Fri, Dec 01, 2017 at 04:49:01PM -0600, Pierre-Louis Bossart wrote:

> >+int sdw_master_read_prop(struct sdw_bus *bus)
> >+{
> >+	struct sdw_master_prop *prop = &bus->prop;
> >+	struct fwnode_handle *link;
> >+	unsigned int count = 0;
> >+	char name[32];
> >+	int nval, i;
> >+
> >+	device_property_read_u32(bus->dev,
> >+			"mipi-sdw-sw-interface-revision", &prop->revision);
> >+	device_property_read_u32(bus->dev, "mipi-sdw-master-count", &count);
> >+
> >+	/* Find link handle */
> >+	snprintf(name, sizeof(name),
> >+			"mipi-sdw-link-%d-subproperties", bus->link_id);
> 
> if you follow the DisCo spec, this property is at the controller level,
> isn't there a confusion between controller/master here, and consequently are
> we reading the same things multiple times or using the wrong bus parameter?

Not sure I follow, this one is for a specific master ie a specfic link.
we need to read respective master thru mipi-sdw-link-N-subproperties

> If I look at intel_probe(), there is a clear reference to a link_id, and
> then you set the pointer to this read_prop which reads the number of links,
> which looks like the wrong order. You can't assign a link ID before knowing
> how many links there are - or you may be unable to detect issues.

Sorry I dont follow this part. FWIW, when master driver is enumerated it
know the link_id value and then sets the read_prop and then these are read.

Here we are reading "a specific link property" with the knowledge of link_id
value...

> >+	fwnode_property_read_u32(link, "mipi-sdw-default-frame-rate",
> >+				&prop->default_frame_rate);
> >+	fwnode_property_read_u32(link, "mipi-sdw-default-frame-row-size",
> >+				&prop->default_row);
> >+	fwnode_property_read_u32(link, "mipi-sdw-default-frame-col-size",
> 
> This is fine, just wondering if we should warnings if the values make no
> sense, e.g. the DisCo spec states in Note1 page 15 that the values are
> interrelated.

I think we discussed in past and that would kind of form the firmware
validation. We check all the values to see if firmware gave us sane values..

> >+	/*
> >+	 * Based on each DPn port, get source and sink dpn properties.
> >+	 * Also, some ports can operate as both source or sink.
> >+	 */
> >+
> >+	/* Allocate memory for set bits in port lists */
> >+	nval = hweight32(prop->source_ports);
> >+	num_of_ports += nval;
> 
> this and...
> 
> >+	prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> >+				sizeof(*prop->src_dpn_prop), GFP_KERNEL);
> >+	if (!prop->src_dpn_prop)
> >+		return -ENOMEM;
> >+
> >+	/* Read dpn properties for source port(s) */
> >+	sdw_slave_read_dpn(slave, prop->src_dpn_prop, nval,
> >+			prop->source_ports, "source");
> >+
> >+	nval = hweight32(prop->sink_ports);
> >+	num_of_ports += nval;
> 
> ... this is no longer needed since...
> 
> >+	prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> >+				sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
> >+	if (!prop->sink_dpn_prop)
> >+		return -ENOMEM;
> >+
> >+	/* Read dpn properties for sink port(s) */
> >+	sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
> >+			prop->sink_ports, "sink");
> >+
> >+	/* some ports are bidirectional so check total ports by ORing */
> >+	nval = prop->source_ports | prop->sink_ports;
> >+	num_of_ports = hweight32(nval) + 1; /* add 1 for DP0 */
> 
> ... you reassign the value here. That was one earlier feedback from me but
> you left the variable incrementation in the code.

This seems to have artifact of merge conflicts as I clearly remember removing
this, thanks for pointing will remove these..

> >+/**
> >+ * enum sdw_clk_stop_mode - Clock Stop modes
> >+ * @SDW_CLK_STOP_MODE0: Slave can continue operation seamlessly on clock
> >+ * restart
> >+ * @SDW_CLK_STOP_MODE1: Slave may have entered a deeper power-saving mode,
> >+ * not capable of continuing operation seamlessly when the clock restarts
> >+ */
> >+enum sdw_clk_stop_mode {
> >+	SDW_CLK_STOP_MODE0 = 1,
> >+	SDW_CLK_STOP_MODE1 = 2,
> 
> why not 0 and 1?

why not 1 and 2 :D

I think it was to ensure we have a non zero value, but am not sure, will
check though..
Pierre-Louis Bossart Dec. 4, 2017, 2:50 a.m. UTC | #3
On 12/3/17 10:52 AM, Vinod Koul wrote:
> On Fri, Dec 01, 2017 at 04:49:01PM -0600, Pierre-Louis Bossart wrote:
> 
>>> +int sdw_master_read_prop(struct sdw_bus *bus)
>>> +{
>>> +	struct sdw_master_prop *prop = &bus->prop;
>>> +	struct fwnode_handle *link;
>>> +	unsigned int count = 0;
>>> +	char name[32];
>>> +	int nval, i;
>>> +
>>> +	device_property_read_u32(bus->dev,
>>> +			"mipi-sdw-sw-interface-revision", &prop->revision);
>>> +	device_property_read_u32(bus->dev, "mipi-sdw-master-count", &count);
>>> +
>>> +	/* Find link handle */
>>> +	snprintf(name, sizeof(name),
>>> +			"mipi-sdw-link-%d-subproperties", bus->link_id);
>>
>> if you follow the DisCo spec, this property is at the controller level,
>> isn't there a confusion between controller/master here, and consequently are
>> we reading the same things multiple times or using the wrong bus parameter?
> 
> Not sure I follow, this one is for a specific master ie a specfic link.
> we need to read respective master thru mipi-sdw-link-N-subproperties
> 
>> If I look at intel_probe(), there is a clear reference to a link_id, and
>> then you set the pointer to this read_prop which reads the number of links,
>> which looks like the wrong order. You can't assign a link ID before knowing
>> how many links there are - or you may be unable to detect issues.
> 
> Sorry I dont follow this part. FWIW, when master driver is enumerated it
> know the link_id value and then sets the read_prop and then these are read.
> 
> Here we are reading "a specific link property" with the knowledge of link_id
> value...

the sdw_master-count is at the controller level, and the linkid has to 
be < master_count.

The fact that you are reading this property for each master instance is 
the problem.

> 
>>> +	fwnode_property_read_u32(link, "mipi-sdw-default-frame-rate",
>>> +				&prop->default_frame_rate);
>>> +	fwnode_property_read_u32(link, "mipi-sdw-default-frame-row-size",
>>> +				&prop->default_row);
>>> +	fwnode_property_read_u32(link, "mipi-sdw-default-frame-col-size",
>>
>> This is fine, just wondering if we should warnings if the values make no
>> sense, e.g. the DisCo spec states in Note1 page 15 that the values are
>> interrelated.
> 
> I think we discussed in past and that would kind of form the firmware
> validation. We check all the values to see if firmware gave us sane values..
> 
>>> +	/*
>>> +	 * Based on each DPn port, get source and sink dpn properties.
>>> +	 * Also, some ports can operate as both source or sink.
>>> +	 */
>>> +
>>> +	/* Allocate memory for set bits in port lists */
>>> +	nval = hweight32(prop->source_ports);
>>> +	num_of_ports += nval;
>>
>> this and...
>>
>>> +	prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
>>> +				sizeof(*prop->src_dpn_prop), GFP_KERNEL);
>>> +	if (!prop->src_dpn_prop)
>>> +		return -ENOMEM;
>>> +
>>> +	/* Read dpn properties for source port(s) */
>>> +	sdw_slave_read_dpn(slave, prop->src_dpn_prop, nval,
>>> +			prop->source_ports, "source");
>>> +
>>> +	nval = hweight32(prop->sink_ports);
>>> +	num_of_ports += nval;
>>
>> ... this is no longer needed since...
>>
>>> +	prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
>>> +				sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
>>> +	if (!prop->sink_dpn_prop)
>>> +		return -ENOMEM;
>>> +
>>> +	/* Read dpn properties for sink port(s) */
>>> +	sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
>>> +			prop->sink_ports, "sink");
>>> +
>>> +	/* some ports are bidirectional so check total ports by ORing */
>>> +	nval = prop->source_ports | prop->sink_ports;
>>> +	num_of_ports = hweight32(nval) + 1; /* add 1 for DP0 */
>>
>> ... you reassign the value here. That was one earlier feedback from me but
>> you left the variable incrementation in the code.
> 
> This seems to have artifact of merge conflicts as I clearly remember removing
> this, thanks for pointing will remove these..
> 
>>> +/**
>>> + * enum sdw_clk_stop_mode - Clock Stop modes
>>> + * @SDW_CLK_STOP_MODE0: Slave can continue operation seamlessly on clock
>>> + * restart
>>> + * @SDW_CLK_STOP_MODE1: Slave may have entered a deeper power-saving mode,
>>> + * not capable of continuing operation seamlessly when the clock restarts
>>> + */
>>> +enum sdw_clk_stop_mode {
>>> +	SDW_CLK_STOP_MODE0 = 1,
>>> +	SDW_CLK_STOP_MODE1 = 2,
>>
>> why not 0 and 1?
> 
> why not 1 and 2 :D
> 
> I think it was to ensure we have a non zero value, but am not sure, will
> check though..

I don't think the value matter and you should use the same conventions 
for such enums.

>
diff mbox

Patch

diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
index c875e434f8b3..bcde0d26524c 100644
--- a/drivers/soundwire/Makefile
+++ b/drivers/soundwire/Makefile
@@ -3,5 +3,5 @@ 
 #
 
 #Bus Objs
-soundwire-bus-objs := bus_type.o bus.o slave.o
+soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o
 obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 0f89b2f36938..507ae85ad58e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -25,6 +25,14 @@  int sdw_add_bus_master(struct sdw_bus *bus)
 	mutex_init(&bus->bus_lock);
 	INIT_LIST_HEAD(&bus->slaves);
 
+	if (bus->ops->read_prop) {
+		ret = bus->ops->read_prop(bus);
+		if (ret < 0) {
+			dev_err(bus->dev, "Bus read properties failed:%d", ret);
+			return ret;
+		}
+	}
+
 	/*
 	 * Device numbers in SoundWire are 0 thru 15 with 0 being
 	 * Enumeration device number and 15 broadcast device number. So
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index 8d8dcc68e9a8..d5f3a70c06b0 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -77,6 +77,8 @@  static int sdw_drv_probe(struct device *dev)
 	if (!id)
 		return -ENODEV;
 
+	slave->ops = drv->ops;
+
 	/*
 	 * attach to power domain but don't turn on (last arg)
 	 */
@@ -89,7 +91,26 @@  static int sdw_drv_probe(struct device *dev)
 		}
 	}
 
-	return ret;
+	if (ret)
+		return ret;
+
+	/* device is probed so let's read the properties now */
+	if (slave->ops && slave->ops->read_prop)
+		slave->ops->read_prop(slave);
+
+	/*
+	 * Check for valid clk_stop_timeout, use DisCo worst case value of
+	 * 300ms
+	 *
+	 * TODO: check the timeouts and driver removal case
+	 */
+	if (slave->prop.clk_stop_timeout == 0)
+		slave->prop.clk_stop_timeout = 300;
+
+	slave->bus->clk_stop_timeout = max_t(u32, slave->bus->clk_stop_timeout,
+					slave->prop.clk_stop_timeout);
+
+	return 0;
 }
 
 static int sdw_drv_remove(struct device *dev)
diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
new file mode 100644
index 000000000000..b8870d4a25ba
--- /dev/null
+++ b/drivers/soundwire/mipi_disco.c
@@ -0,0 +1,374 @@ 
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2015-17 Intel Corporation.
+
+/*
+ * MIPI Discovery And Configuration (DisCo) Specification for SoundWire
+ * specifies properties to be implemented for SoundWire Masters and Slaves.
+ * The DisCo spec doesn't mandate these properties. However, SDW bus cannot
+ * work without knowing these values.
+ *
+ * The helper functions read the Master and Slave properties. Implementers
+ * of Master or Slave drivers can use any of the below three mechanisms:
+ *    a) Use these APIs here as .read_prop() callback for Master and Slave
+ *    b) Implement own methods and set those as .read_prop(), but invoke
+ *    APIs in this file for generic read and override the values with
+ *    platform specific data
+ *    c) Implement ones own methods which do not use anything provided
+ *    here
+ */
+
+#include <linux/device.h>
+#include <linux/property.h>
+#include <linux/mod_devicetable.h>
+#include <linux/soundwire/sdw.h>
+#include "bus.h"
+
+/**
+ * sdw_master_read_prop() - Read Master properties
+ * @bus: SDW bus instance
+ */
+int sdw_master_read_prop(struct sdw_bus *bus)
+{
+	struct sdw_master_prop *prop = &bus->prop;
+	struct fwnode_handle *link;
+	unsigned int count = 0;
+	char name[32];
+	int nval, i;
+
+	device_property_read_u32(bus->dev,
+			"mipi-sdw-sw-interface-revision", &prop->revision);
+	device_property_read_u32(bus->dev, "mipi-sdw-master-count", &count);
+
+	/* Find link handle */
+	snprintf(name, sizeof(name),
+			"mipi-sdw-link-%d-subproperties", bus->link_id);
+
+	link = device_get_named_child_node(bus->dev, name);
+	if (!link) {
+		dev_err(bus->dev, "Link node %s not found\n", name);
+		return -EIO;
+	}
+
+	if (fwnode_property_read_bool(link,
+			"mipi-sdw-clock-stop-mode0-supported") == true)
+		prop->clk_stop_mode = SDW_CLK_STOP_MODE0;
+
+	if (fwnode_property_read_bool(link,
+			"mipi-sdw-clock-stop-mode1-supported") == true)
+		prop->clk_stop_mode |= SDW_CLK_STOP_MODE1;
+
+	fwnode_property_read_u32(link,
+			"mipi-sdw-max-clock-frequency", &prop->max_freq);
+
+	nval = fwnode_property_read_u32_array(link,
+			"mipi-sdw-clock-frequencies-supported", NULL, 0);
+	if (nval > 0)
+		prop->num_freq = nval;
+
+	if (prop->num_freq) {
+		prop->freq = devm_kcalloc(bus->dev, nval,
+					sizeof(*prop->freq), GFP_KERNEL);
+		if (!prop->freq)
+			return -ENOMEM;
+
+		fwnode_property_read_u32_array(link,
+				"mipi-sdw-clock-frequencies-supported",
+				prop->freq, nval);
+
+		/*
+		 * Check the frequencies supported. If FW doesn't provide max
+		 * freq, then populate here by checking values.
+		 */
+		if (!prop->max_freq) {
+			prop->max_freq = prop->freq[0];
+			for (i = 1; i < prop->num_freq; i++) {
+				if (prop->freq[i] > prop->max_freq)
+					prop->max_freq = prop->freq[i];
+			}
+		}
+	}
+
+	nval = fwnode_property_read_u32_array(link,
+			"mipi-sdw-supported-clock-gears", NULL, 0);
+	if (nval > 0)
+		prop->num_clk_gears = nval;
+
+	if (prop->num_clk_gears) {
+		prop->clk_gears = devm_kcalloc(bus->dev, nval,
+				sizeof(*prop->clk_gears), GFP_KERNEL);
+		if (!prop->clk_gears)
+			return -ENOMEM;
+
+		fwnode_property_read_u32_array(link,
+				"mipi-sdw-supported-clock-gears",
+				prop->clk_gears, nval);
+	}
+
+	fwnode_property_read_u32(link, "mipi-sdw-default-frame-rate",
+				&prop->default_frame_rate);
+	fwnode_property_read_u32(link, "mipi-sdw-default-frame-row-size",
+				&prop->default_row);
+	fwnode_property_read_u32(link, "mipi-sdw-default-frame-col-size",
+				&prop->default_col);
+	prop->dynamic_frame =  fwnode_property_read_bool(link,
+				"mipi-sdw-dynamic-frame-shape");
+	fwnode_property_read_u32(link, "mipi-sdw-command-error-threshold",
+				&prop->err_threshold);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdw_master_read_prop);
+
+static int sdw_slave_read_dpn(struct sdw_slave *slave,
+		struct sdw_dpn_prop *dpn, int count, int ports, char *type)
+{
+	struct fwnode_handle *node;
+	u32 bit, i = 0, nval;
+	unsigned long addr;
+	char name[40];
+
+	addr = ports;
+	/* valid ports are 1 to 14 so apply mask */
+	addr &= GENMASK(14, 1);
+
+	for_each_set_bit(bit, &addr, 32) {
+		snprintf(name, sizeof(name),
+			"mipi-sdw-dp-%d-%s-subproperties", bit, type);
+
+		dpn[i].num = bit;
+
+		node = device_get_named_child_node(&slave->dev, name);
+		if (!node) {
+			dev_err(&slave->dev, "%s dpN not found\n", name);
+			return -EIO;
+		}
+
+		fwnode_property_read_u32(node, "mipi-sdw-port-max-wordlength",
+						&dpn[i].max_word);
+		fwnode_property_read_u32(node, "mipi-sdw-port-min-wordlength",
+						&dpn[i].min_word);
+
+		nval = fwnode_property_read_u32_array(node,
+				"mipi-sdw-port-wordlength-configs", NULL, 0);
+		if (nval > 0)
+			dpn[i].num_words = nval;
+
+		if (dpn[i].num_words) {
+			dpn[i].words = devm_kcalloc(&slave->dev, nval,
+				sizeof(*dpn[i].words), GFP_KERNEL);
+			if (!dpn[i].words)
+				return -ENOMEM;
+
+			fwnode_property_read_u32_array(node,
+					"mipi-sdw-port-wordlength-configs",
+					&dpn[i].num_words, nval);
+		}
+
+		fwnode_property_read_u32(node, "mipi-sdw-data-port-type",
+				&dpn[i].type);
+		fwnode_property_read_u32(node,
+				"mipi-sdw-max-grouping-supported",
+				&dpn[i].max_grouping);
+		dpn[i].simple_ch_prep_sm = fwnode_property_read_bool(node,
+				"mipi-sdw-simplified-channelprepare-sm");
+		fwnode_property_read_u32(node,
+				"mipi-sdw-port-channelprepare-timeout",
+				&dpn[i].ch_prep_timeout);
+		fwnode_property_read_u32(node,
+				"mipi-sdw-imp-def-dpn-interrupts-supported",
+				&dpn[i].device_interrupts);
+		fwnode_property_read_u32(node, "mipi-sdw-min-channel-number",
+				&dpn[i].min_ch);
+		fwnode_property_read_u32(node, "mipi-sdw-max-channel-number",
+				&dpn[i].max_ch);
+
+		nval = fwnode_property_read_u32_array(node,
+				"mipi-sdw-channel-number-list", NULL, 0);
+		if (nval > 0)
+			dpn[i].num_ch = nval;
+
+		if (dpn[i].num_ch) {
+			dpn[i].ch = devm_kcalloc(&slave->dev, nval,
+					sizeof(*dpn[i].ch), GFP_KERNEL);
+			if (!dpn[i].ch)
+				return -ENOMEM;
+
+			fwnode_property_read_u32_array(node,
+					"mipi-sdw-channel-number-list",
+					dpn[i].ch, nval);
+		}
+
+		nval = fwnode_property_read_u32_array(node,
+				"mipi-sdw-channel-combination-list", NULL, 0);
+		if (nval > 0)
+			dpn[i].num_ch_combinations = nval;
+
+		if (dpn[i].num_ch_combinations) {
+			dpn[i].ch_combinations = devm_kcalloc(&slave->dev,
+					nval, sizeof(*dpn[i].ch_combinations),
+					GFP_KERNEL);
+			if (!dpn[i].ch_combinations)
+				return -ENOMEM;
+
+			fwnode_property_read_u32_array(node,
+					"mipi-sdw-channel-combination-list",
+					dpn[i].ch_combinations, nval);
+		}
+
+		fwnode_property_read_u32(node,
+				"mipi-sdw-modes-supported", &dpn[i].modes);
+		fwnode_property_read_u32(node, "mipi-sdw-max-async-buffer",
+				&dpn[i].max_async_buffer);
+		dpn[i].block_pack_mode = fwnode_property_read_bool(node,
+				"mipi-sdw-block-packing-mode");
+
+		fwnode_property_read_u32(node, "mipi-sdw-port-encoding-type",
+				&dpn[i].port_encoding);
+
+		/* TODO: Read audio mode */
+
+		i++;
+	}
+
+	return 0;
+}
+
+/**
+ * sdw_slave_read_prop() - Read Slave properties
+ * @slave: SDW Slave
+ */
+int sdw_slave_read_prop(struct sdw_slave *slave)
+{
+	struct sdw_slave_prop *prop = &slave->prop;
+	struct device *dev = &slave->dev;
+	struct fwnode_handle *port;
+	int num_of_ports, nval, i;
+
+	device_property_read_u32(dev, "mipi-sdw-sw-interface-revision",
+				&prop->mipi_revision);
+
+	prop->wake_capable = device_property_read_bool(dev,
+				"mipi-sdw-wake-up-unavailable");
+	prop->wake_capable = !prop->wake_capable;
+
+	prop->test_mode_capable = device_property_read_bool(dev,
+				"mipi-sdw-test-mode-supported");
+
+	prop->clk_stop_mode1 = false;
+	if (device_property_read_bool(dev,
+				"mipi-sdw-clock-stop-mode1-supported"))
+		prop->clk_stop_mode1 = true;
+
+	prop->simple_clk_stop_capable = device_property_read_bool(dev,
+			"mipi-sdw-simplified-clockstopprepare-sm-supported");
+
+	device_property_read_u32(dev, "mipi-sdw-clockstopprepare-timeout",
+			&prop->clk_stop_timeout);
+	device_property_read_u32(dev, "mipi-sdw-slave-channelprepare-timeout",
+			&prop->ch_prep_timeout);
+	device_property_read_u32(dev,
+			"mipi-sdw-clockstopprepare-hard-reset-behavior",
+			&prop->reset_behave);
+
+	prop->high_PHY_capable = device_property_read_bool(dev,
+			"mipi-sdw-highPHY-capable");
+	prop->paging_support = device_property_read_bool(dev,
+			"mipi-sdw-paging-support");
+	prop->bank_delay_support = device_property_read_bool(dev,
+			"mipi-sdw-bank-delay-support");
+	device_property_read_u32(dev,
+			"mipi-sdw-port15-read-behavior", &prop->p15_behave);
+	device_property_read_u32(dev, "mipi-sdw-master-count",
+				&prop->master_count);
+
+	device_property_read_u32(dev, "mipi-sdw-source-port-list",
+				&prop->source_ports);
+	device_property_read_u32(dev, "mipi-sdw-sink-port-list",
+				&prop->sink_ports);
+
+	/* Read dp0 properties */
+	port = device_get_named_child_node(dev, "mipi-sdw-dp-0-subproperties");
+	if (!port) {
+		dev_err(dev, "DP0 node not found!!\n");
+		return -EIO;
+	}
+
+	prop->dp0_prop = devm_kzalloc(&slave->dev,
+			sizeof(*prop->dp0_prop), GFP_KERNEL);
+	if (!prop->dp0_prop)
+		return -ENOMEM;
+
+	fwnode_property_read_u32(port, "mipi-sdw-port-max-wordlength",
+			&prop->dp0_prop->max_word);
+	fwnode_property_read_u32(port, "mipi-sdw-port-min-wordlength",
+			&prop->dp0_prop->min_word);
+	nval = fwnode_property_read_u32_array(port,
+			"mipi-sdw-port-wordlength-configs", NULL, 0);
+	if (nval > 0)
+		prop->dp0_prop->num_words = nval;
+
+	if (prop->dp0_prop->num_words) {
+		prop->dp0_prop->words = devm_kcalloc(&slave->dev, nval,
+				sizeof(*prop->dp0_prop->words), GFP_KERNEL);
+		if (!prop->dp0_prop->words)
+			return -ENOMEM;
+
+		fwnode_property_read_u32_array(port,
+				"mipi-sdw-port-wordlength-configs",
+				prop->dp0_prop->words, nval);
+	}
+
+	prop->dp0_prop->flow_controlled = fwnode_property_read_bool(port,
+			"mipi-sdw-bra-flow-controlled");
+
+	prop->dp0_prop->simple_ch_prep_sm = fwnode_property_read_bool(port,
+			"mipi-sdw-simplified-channel-prepare-sm");
+
+	prop->dp0_prop->device_interrupts = fwnode_property_read_bool(port,
+			"mipi-sdw-imp-def-dp0-interrupts-supported");
+
+	/*
+	 * Based on each DPn port, get source and sink dpn properties.
+	 * Also, some ports can operate as both source or sink.
+	 */
+
+	/* Allocate memory for set bits in port lists */
+	nval = hweight32(prop->source_ports);
+	num_of_ports += nval;
+	prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
+				sizeof(*prop->src_dpn_prop), GFP_KERNEL);
+	if (!prop->src_dpn_prop)
+		return -ENOMEM;
+
+	/* Read dpn properties for source port(s) */
+	sdw_slave_read_dpn(slave, prop->src_dpn_prop, nval,
+			prop->source_ports, "source");
+
+	nval = hweight32(prop->sink_ports);
+	num_of_ports += nval;
+	prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
+				sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
+	if (!prop->sink_dpn_prop)
+		return -ENOMEM;
+
+	/* Read dpn properties for sink port(s) */
+	sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
+			prop->sink_ports, "sink");
+
+	/* some ports are bidirectional so check total ports by ORing */
+	nval = prop->source_ports | prop->sink_ports;
+	num_of_ports = hweight32(nval) + 1; /* add 1 for DP0 */
+
+	/* Allocate port_ready based on num_of_ports */
+	slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
+				sizeof(*slave->port_ready), GFP_KERNEL);
+	if (!slave->port_ready)
+		return -ENOMEM;
+
+	/* Initialize completion */
+	for (i = 0; i < num_of_ports; i++)
+		init_completion(&slave->port_ready[i]);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdw_slave_read_prop);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 7e9579941c66..ddfae18f4306 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -32,6 +32,252 @@  enum sdw_slave_status {
 };
 
 /*
+ * SDW properties, defined in MIPI DisCo spec v1.0
+ */
+enum sdw_clk_stop_reset_behave {
+	SDW_CLK_STOP_KEEP_STATUS = 1,
+};
+
+/**
+ * enum sdw_p15_behave - Slave Port 15 behaviour when the Master attempts a
+ * read
+ * @SDW_P15_READ_IGNORED: Read is ignored
+ * @SDW_P15_CMD_OK: Command is ok
+ */
+enum sdw_p15_behave {
+	SDW_P15_READ_IGNORED = 0,
+	SDW_P15_CMD_OK = 1,
+};
+
+/**
+ * enum sdw_dpn_type - Data port types
+ * @SDW_DPN_FULL: Full Data Port is supported
+ * @SDW_DPN_SIMPLE: Simplified Data Port as defined in spec.
+ * DPN_SampleCtrl2, DPN_OffsetCtrl2, DPN_HCtrl and DPN_BlockCtrl3
+ * are not implemented.
+ * @SDW_DPN_REDUCED: Reduced Data Port as defined in spec.
+ * DPN_SampleCtrl2, DPN_HCtrl are not implemented.
+ */
+enum sdw_dpn_type {
+	SDW_DPN_FULL = 0,
+	SDW_DPN_SIMPLE = 1,
+	SDW_DPN_REDUCED = 2,
+};
+
+/**
+ * enum sdw_clk_stop_mode - Clock Stop modes
+ * @SDW_CLK_STOP_MODE0: Slave can continue operation seamlessly on clock
+ * restart
+ * @SDW_CLK_STOP_MODE1: Slave may have entered a deeper power-saving mode,
+ * not capable of continuing operation seamlessly when the clock restarts
+ */
+enum sdw_clk_stop_mode {
+	SDW_CLK_STOP_MODE0 = 1,
+	SDW_CLK_STOP_MODE1 = 2,
+};
+
+/**
+ * struct sdw_dp0_prop - DP0 properties
+ * @max_word: Maximum number of bits in a Payload Channel Sample, 1 to 64
+ * (inclusive)
+ * @min_word: Minimum number of bits in a Payload Channel Sample, 1 to 64
+ * (inclusive)
+ * @num_words: number of wordlengths supported
+ * @words: wordlengths supported
+ * @flow_controlled: Slave implementation results in an OK_NotReady
+ * response
+ * @simple_ch_prep_sm: If channel prepare sequence is required
+ * @device_interrupts: If implementation-defined interrupts are supported
+ *
+ * The wordlengths are specified by Spec as max, min AND number of
+ * discrete values, implementation can define based on the wordlengths they
+ * support
+ */
+struct sdw_dp0_prop {
+	u32 max_word;
+	u32 min_word;
+	u32 num_words;
+	u32 *words;
+	bool flow_controlled;
+	bool simple_ch_prep_sm;
+	bool device_interrupts;
+};
+
+/**
+ * struct sdw_dpn_audio_mode - Audio mode properties for DPn
+ * @bus_min_freq: Minimum bus frequency, in Hz
+ * @bus_max_freq: Maximum bus frequency, in Hz
+ * @bus_num_freq: Number of discrete frequencies supported
+ * @bus_freq: Discrete bus frequencies, in Hz
+ * @min_freq: Minimum sampling frequency, in Hz
+ * @max_freq: Maximum sampling bus frequency, in Hz
+ * @num_freq: Number of discrete sampling frequency supported
+ * @freq: Discrete sampling frequencies, in Hz
+ * @prep_ch_behave: Specifies the dependencies between Channel Prepare
+ * sequence and bus clock configuration
+ * If 0, Channel Prepare can happen at any Bus clock rate
+ * If 1, Channel Prepare sequence shall happen only after Bus clock is
+ * changed to a frequency supported by this mode or compatible modes
+ * described by the next field
+ * @glitchless: Bitmap describing possible glitchless transitions from this
+ * Audio Mode to other Audio Modes
+ */
+struct sdw_dpn_audio_mode {
+	u32 bus_min_freq;
+	u32 bus_max_freq;
+	u32 bus_num_freq;
+	u32 *bus_freq;
+	u32 max_freq;
+	u32 min_freq;
+	u32 num_freq;
+	u32 *freq;
+	u32 prep_ch_behave;
+	u32 glitchless;
+};
+
+/**
+ * struct sdw_dpn_prop - Data Port DPn properties
+ * @num: port number
+ * @max_word: Maximum number of bits in a Payload Channel Sample, 1 to 64
+ * (inclusive)
+ * @min_word: Minimum number of bits in a Payload Channel Sample, 1 to 64
+ * (inclusive)
+ * @num_words: Number of discrete supported wordlengths
+ * @words: Discrete supported wordlength
+ * @type: Data port type. Full, Simplified or Reduced
+ * @max_grouping: Maximum number of samples that can be grouped together for
+ * a full data port
+ * @simple_ch_prep_sm: If the port supports simplified channel prepare state
+ * machine
+ * @ch_prep_timeout: Port-specific timeout value, in milliseconds
+ * @device_interrupts: If set, each bit corresponds to support for
+ * implementation-defined interrupts
+ * @max_ch: Maximum channels supported
+ * @min_ch: Minimum channels supported
+ * @num_ch: Number of discrete channels supported
+ * @ch: Discrete channels supported
+ * @num_ch_combinations: Number of channel combinations supported
+ * @ch_combinations: Channel combinations supported
+ * @modes: SDW mode supported
+ * @max_async_buffer: Number of samples that this port can buffer in
+ * asynchronous modes
+ * @block_pack_mode: Type of block port mode supported
+ * @port_encoding: Payload Channel Sample encoding schemes supported
+ * @audio_modes: Audio modes supported
+ */
+struct sdw_dpn_prop {
+	u32 num;
+	u32 max_word;
+	u32 min_word;
+	u32 num_words;
+	u32 *words;
+	enum sdw_dpn_type type;
+	u32 max_grouping;
+	bool simple_ch_prep_sm;
+	u32 ch_prep_timeout;
+	u32 device_interrupts;
+	u32 max_ch;
+	u32 min_ch;
+	u32 num_ch;
+	u32 *ch;
+	u32 num_ch_combinations;
+	u32 *ch_combinations;
+	u32 modes;
+	u32 max_async_buffer;
+	bool block_pack_mode;
+	u32 port_encoding;
+	struct sdw_dpn_audio_mode *audio_modes;
+};
+
+/**
+ * struct sdw_slave_prop - SoundWire Slave properties
+ * @mipi_revision: Spec version of the implementation
+ * @wake_capable: Wake-up events are supported
+ * @test_mode_capable: If test mode is supported
+ * @clk_stop_mode1: Clock-Stop Mode 1 is supported
+ * @simple_clk_stop_capable: Simple clock mode is supported
+ * @clk_stop_timeout: Worst-case latency of the Clock Stop Prepare State
+ * Machine transitions, in milliseconds
+ * @ch_prep_timeout: Worst-case latency of the Channel Prepare State Machine
+ * transitions, in milliseconds
+ * @reset_behave: Slave keeps the status of the SlaveStopClockPrepare
+ * state machine (P=1 SCSP_SM) after exit from clock-stop mode1
+ * @high_PHY_capable: Slave is HighPHY capable
+ * @paging_support: Slave implements paging registers SCP_AddrPage1 and
+ * SCP_AddrPage2
+ * @bank_delay_support: Slave implements bank delay/bridge support registers
+ * SCP_BankDelay and SCP_NextFrame
+ * @p15_behave: Slave behavior when the Master attempts a read to the Port15
+ * alias
+ * @lane_control_support: Slave supports lane control
+ * @master_count: Number of Masters present on this Slave
+ * @source_ports: Bitmap identifying source ports
+ * @sink_ports: Bitmap identifying sink ports
+ * @dp0_prop: Data Port 0 properties
+ * @src_dpn_prop: Source Data Port N properties
+ * @sink_dpn_prop: Sink Data Port N properties
+ */
+struct sdw_slave_prop {
+	u32 mipi_revision;
+	bool wake_capable;
+	bool test_mode_capable;
+	bool clk_stop_mode1;
+	bool simple_clk_stop_capable;
+	u32 clk_stop_timeout;
+	u32 ch_prep_timeout;
+	enum sdw_clk_stop_reset_behave reset_behave;
+	bool high_PHY_capable;
+	bool paging_support;
+	bool bank_delay_support;
+	enum sdw_p15_behave p15_behave;
+	bool lane_control_support;
+	u32 master_count;
+	u32 source_ports;
+	u32 sink_ports;
+	struct sdw_dp0_prop *dp0_prop;
+	struct sdw_dpn_prop *src_dpn_prop;
+	struct sdw_dpn_prop *sink_dpn_prop;
+};
+
+/**
+ * struct sdw_master_prop - Master properties
+ * @revision: MIPI spec version of the implementation
+ * @master_count: Number of masters
+ * @clk_stop_mode: Bitmap for Clock Stop modes supported
+ * @max_freq: Maximum Bus clock frequency, in Hz
+ * @num_clk_gears: Number of clock gears supported
+ * @clk_gears: Clock gears supported
+ * @num_freq: Number of clock frequencies supported, in Hz
+ * @freq: Clock frequencies supported, in Hz
+ * @default_frame_rate: Controller default Frame rate, in Hz
+ * @default_row: Number of rows
+ * @default_col: Number of columns
+ * @dynamic_frame: Dynamic frame supported
+ * @err_threshold: Number of times that software may retry sending a single
+ * command
+ * @dpn_prop: Data Port N properties
+ */
+struct sdw_master_prop {
+	u32 revision;
+	u32 master_count;
+	enum sdw_clk_stop_mode clk_stop_mode;
+	u32 max_freq;
+	u32 num_clk_gears;
+	u32 *clk_gears;
+	u32 num_freq;
+	u32 *freq;
+	u32 default_frame_rate;
+	u32 default_row;
+	u32 default_col;
+	bool dynamic_frame;
+	u32 err_threshold;
+	struct sdw_dpn_prop *dpn_prop;
+};
+
+int sdw_master_read_prop(struct sdw_bus *bus);
+int sdw_slave_read_prop(struct sdw_slave *slave);
+
+/*
  * SDW Slave Structures and APIs
  */
 
@@ -55,12 +301,23 @@  struct sdw_slave_id {
 };
 
 /**
+ * struct sdw_slave_ops - Slave driver callback ops
+ * @read_prop: Read Slave properties
+ */
+struct sdw_slave_ops {
+	int (*read_prop)(struct sdw_slave *sdw);
+};
+
+/**
  * struct sdw_slave - SoundWire Slave
  * @id: MIPI device ID
  * @dev: Linux device
  * @status: Status reported by the Slave
  * @bus: Bus handle
+ * @ops: Slave callback ops
+ * @prop: Slave properties
  * @node: node for bus list
+ * @port_ready: Port ready completion flag for each Slave port
  * @dev_num: Device Number assigned by Bus
  */
 struct sdw_slave {
@@ -68,7 +325,10 @@  struct sdw_slave {
 	struct device dev;
 	enum sdw_slave_status status;
 	struct sdw_bus *bus;
+	const struct sdw_slave_ops *ops;
+	struct sdw_slave_prop prop;
 	struct list_head node;
+	struct completion *port_ready;
 	u16 dev_num;
 };
 
@@ -100,6 +360,14 @@  int sdw_handle_slave_status(struct sdw_bus *bus,
  */
 
 /**
+ * struct sdw_master_ops - Master driver ops
+ * @read_prop: Read Master properties
+ */
+struct sdw_master_ops {
+	int (*read_prop)(struct sdw_bus *bus);
+};
+
+/**
  * struct sdw_bus - SoundWire bus
  * @dev: Master linux device
  * @link_id: Link id number, can be 0 to N, unique for each Master
@@ -107,6 +375,9 @@  int sdw_handle_slave_status(struct sdw_bus *bus,
  * @assigned: Bitmap for Slave device numbers.
  * Bit set implies used number, bit clear implies unused number.
  * @bus_lock: bus lock
+ * @ops: Master callback ops
+ * @prop: Master properties
+ * @clk_stop_timeout: Clock stop timeout computed
  */
 struct sdw_bus {
 	struct device *dev;
@@ -114,6 +385,9 @@  struct sdw_bus {
 	struct list_head slaves;
 	DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
 	struct mutex bus_lock;
+	const struct sdw_master_ops *ops;
+	struct sdw_master_prop prop;
+	unsigned int clk_stop_timeout;
 };
 
 int sdw_add_bus_master(struct sdw_bus *bus);