lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 4 May 2019 08:54:44 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        tiwai@...e.de, broonie@...nel.org, vkoul@...nel.org,
        liam.r.girdwood@...ux.intel.com, jank@...ence.com, joe@...ches.com,
        srinivas.kandagatla@...aro.org,
        Sanyog Kale <sanyog.r.kale@...el.com>
Subject: Re: [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@...ux.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

Powered by blists - more mailing lists