Re: [alsa-devel] [RFC PATCH 2/7] soundwire: add Slave sysfs support

On Fri, May 03, 2019 at 08:00:25PM -0500, Pierre-Louis Bossart wrote:

Add DisCo Slave properties as device attributes. Add a device for Data Port 0 which adds Dp0 properties as attributes. Add devices for Source and Sink Data Ports, and add Dp-N properties as attributes.

The Slave, DP0 and DPn cases are intentionally handled in separate files to avoid conflicts with attributes having the same names at different levels.

Audio modes are not supported for now. Depending on the discussions the SoundWire Device Class, we may add it later as is or follow the new specification.

Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah

Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

drivers/soundwire/Makefile | 2 +- drivers/soundwire/bus.c | 2 + drivers/soundwire/bus.h | 2 + drivers/soundwire/bus_type.c | 5 + drivers/soundwire/slave.c | 1 + drivers/soundwire/sysfs.c | 213 ++++++++++++++++++++++++++++ drivers/soundwire/sysfs_local.h | 42 ++++++ drivers/soundwire/sysfs_slave_dp0.c | 112 +++++++++++++++ drivers/soundwire/sysfs_slave_dpn.c | 168 ++++++++++++++++++++++ include/linux/soundwire/sdw.h | 5 + 10 files changed, 551 insertions(+), 1 deletion(-) create mode 100644 drivers/soundwire/sysfs_local.h create mode 100644 drivers/soundwire/sysfs_slave_dp0.c create mode 100644 drivers/soundwire/sysfs_slave_dpn.c

diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 787f1cbf342c..a72a29731a28 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -4,7 +4,7 @@

#Bus Objs soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o \

• 	sysfs.o
  

• 	sysfs.o sysfs_slave_dp0.o sysfs_slave_dpn.o
  

obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o

#Cadence Objs diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 38de7071e135..dd9181693554 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -113,6 +113,8 @@ static int sdw_delete_slave(struct device *dev, void *data) struct sdw_slave *slave = dev_to_sdw_dev(dev); struct sdw_bus *bus = slave->bus;

• sdw_sysfs_slave_exit(slave);

• mutex_lock(&bus->bus_lock);

  if (slave->dev_num) /* clear dev_num if assigned */

diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index 3048ca153f22..0707e68a8d21 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -18,6 +18,8 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus) void sdw_extract_slave_id(struct sdw_bus *bus, u64 addr, struct sdw_slave_id *id);

+extern const struct attribute_group *sdw_slave_dev_attr_groups[];

enum { SDW_MSG_FLAG_READ = 0, SDW_MSG_FLAG_WRITE, diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 2655602f0cfb..f68fe45c1037 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -97,6 +97,11 @@ static int sdw_drv_probe(struct device *dev) if (slave->ops && slave->ops->read_prop) slave->ops->read_prop(slave);

• /* init the sysfs as we have properties now */
• ret = sdw_sysfs_slave_init(slave);
• if (ret < 0)

• dev_warn(dev, "Slave sysfs init failed:%d\n", ret);
  

• /*
  • Check for valid clk_stop_timeout, use DisCo worst case value of
  • 300ms

diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index f39a5815e25d..bad73a267fdd 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -34,6 +34,7 @@ static int sdw_slave_add(struct sdw_bus *bus, id->class_id, id->unique_id);

slave->dev.release = sdw_slave_release;

• slave->dev.groups = sdw_slave_dev_attr_groups; slave->dev.bus = &sdw_bus_type; slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED;

diff --git a/drivers/soundwire/sysfs.c b/drivers/soundwire/sysfs.c index 7b6c3826a73a..734e2c8bc5cd 100644 --- a/drivers/soundwire/sysfs.c +++ b/drivers/soundwire/sysfs.c @@ -8,6 +8,7 @@ #include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_type.h> #include "bus.h" +#include "sysfs_local.h"

struct sdw_master_sysfs { struct device dev; @@ -160,3 +161,215 @@ void sdw_sysfs_bus_exit(struct sdw_bus *bus) put_device(&master->dev); bus->sysfs = NULL; }

+/*

• • Slave sysfs
• */

+/*

• • The sysfs for Slave reflects the MIPI description as given
• • in the MIPI DisCo spec
• • Base file is device
• • |---- mipi_revision
• • |---- wake_capable
• • |---- test_mode_capable
• • |---- simple_clk_stop_capable
• • |---- clk_stop_timeout
• • |---- ch_prep_timeout
• • |---- reset_behave
• • |---- high_PHY_capable
• • |---- paging_support
• • |---- bank_delay_support
• • |---- p15_behave
• • |---- master_count
• • |---- source_ports
• • |---- sink_ports
• • |---- dp0

• • |---- max_word
    

• • |---- min_word
    

• • |---- words
    

• • |---- flow_controlled
    

• • |---- simple_ch_prep_sm
    

• • |---- device_interrupts
    

• • |---- dpN

• • |---- max_word
    

• • |---- min_word
    

• • |---- words
    

• • |---- type
    

• • |---- max_grouping
    

• • |---- simple_ch_prep_sm
    

• • |---- ch_prep_timeout
    

• • |---- device_interrupts
    

• • |---- max_ch
    

• • |---- min_ch
    

• • |---- ch
    

• • |---- ch_combinations
    

• • |---- modes
    

• • |---- max_async_buffer
    

• • |---- block_pack_mode
    

• • |---- port_encoding
    

• • |---- audio_modeM
    

• • 		|---- bus_min_freq
    

• • 		|---- bus_max_freq
    

• • 		|---- bus_freq
    

• • 		|---- max_freq
    

• • 		|---- min_freq
    

• • 		|---- freq
    

• • 		|---- prep_ch_behave
    

• • 		|---- glitchless
    

• */

+#define sdw_slave_attr(field, format_string) \ +static ssize_t field##_show(struct device *dev, \

• 	    struct device_attribute *attr,	\
  

• 	    char *buf)				\
  

+{ \

• struct sdw_slave *slave = dev_to_sdw_dev(dev); \
• return sprintf(buf, format_string, slave->prop.field); \

+} \ +static DEVICE_ATTR_RO(field)

+sdw_slave_attr(mipi_revision, "0x%x\n"); +sdw_slave_attr(wake_capable, "%d\n"); +sdw_slave_attr(test_mode_capable, "%d\n"); +sdw_slave_attr(clk_stop_mode1, "%d\n"); +sdw_slave_attr(simple_clk_stop_capable, "%d\n"); +sdw_slave_attr(clk_stop_timeout, "%d\n"); +sdw_slave_attr(ch_prep_timeout, "%d\n"); +sdw_slave_attr(reset_behave, "%d\n"); +sdw_slave_attr(high_PHY_capable, "%d\n"); +sdw_slave_attr(paging_support, "%d\n"); +sdw_slave_attr(bank_delay_support, "%d\n"); +sdw_slave_attr(p15_behave, "%d\n"); +sdw_slave_attr(master_count, "%d\n"); +sdw_slave_attr(source_ports, "%d\n"); +sdw_slave_attr(sink_ports, "%d\n");

+static ssize_t modalias_show(struct device *dev,

• 	     struct device_attribute *attr, char *buf)
  

+{

• struct sdw_slave *slave = dev_to_sdw_dev(dev);
• return sdw_slave_modalias(slave, buf, 256);

+} +static DEVICE_ATTR_RO(modalias);

+static struct attribute *slave_dev_attrs[] = {

• &dev_attr_mipi_revision.attr,
• &dev_attr_wake_capable.attr,
• &dev_attr_test_mode_capable.attr,
• &dev_attr_clk_stop_mode1.attr,
• &dev_attr_simple_clk_stop_capable.attr,
• &dev_attr_clk_stop_timeout.attr,
• &dev_attr_ch_prep_timeout.attr,
• &dev_attr_reset_behave.attr,
• &dev_attr_high_PHY_capable.attr,
• &dev_attr_paging_support.attr,
• &dev_attr_bank_delay_support.attr,
• &dev_attr_p15_behave.attr,
• &dev_attr_master_count.attr,
• &dev_attr_source_ports.attr,
• &dev_attr_sink_ports.attr,
• &dev_attr_modalias.attr,
• NULL,

+};

+static struct attribute_group sdw_slave_dev_attr_group = {

• .attrs = slave_dev_attrs,

+};

+const struct attribute_group *sdw_slave_dev_attr_groups[] = {

• &sdw_slave_dev_attr_group,
• NULL

+};

ATTRIBUTE_GROUP()?

+int sdw_sysfs_slave_init(struct sdw_slave *slave) +{

• struct sdw_slave_sysfs *sysfs;
• unsigned int src_dpns, sink_dpns, i, j;
• int err;
• if (slave->sysfs) {

• dev_err(&slave->dev, "SDW Slave sysfs is already initialized\n");
  

• err = -EIO;
  

• goto err_ret;
  

• }
• sysfs = kzalloc(sizeof(*sysfs), GFP_KERNEL);

Same question as patch 1, why a new device?

thanks,

greg k-h