Re: [RFC PATCH 2/7] soundwire: add Slave sysfs support
From: Greg KH
Date: Sat May 04 2019 - 02:55:20 EST
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@xxxxxxxxxxxxxxx>
> ---
> 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