Re: [PATCH] soundwire: add slave status as a sysfs file

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 9/10/20 9:03 AM, Vinod Koul wrote:
> SoundWire Slave status is not reported in the sysfs, so add this file
> with string values for status to userspace. Users can look into this
> file to see the Slave status at that point of time.

As I explained in my other email, this doesn't work in the case where a 
device does not have a driver or is not described in platform firmware: 
sysfs entries are only added *after* the driver probe.

We've need a separate sysfs attr group for this, added in 
sdw_slave_add() and separate from the attributes added after probe.

> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> On RB3 it shows as:
> root@linaro-alip:/sys/bus/soundwire/devices# cat sdw:0:217:2010:0:1/status
> Attached
> root@linaro-alip:/sys/bus/soundwire/devices# cat sdw:0:217:2010:0:2/status
> Attached
> 
>   .../ABI/testing/sysfs-bus-soundwire-slave       |  7 +++++++
>   drivers/soundwire/sysfs_slave.c                 | 17 +++++++++++++++++
>   include/linux/soundwire/sdw.h                   |  2 ++
>   3 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-soundwire-slave b/Documentation/ABI/testing/sysfs-bus-soundwire-slave
> index db4c9511d1aa..8a64f8e9079a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-soundwire-slave
> +++ b/Documentation/ABI/testing/sysfs-bus-soundwire-slave
> @@ -89,3 +89,10 @@ Description:	SoundWire Slave Data Source/Sink Port-N DisCo properties.
>   		for SoundWire. They define various properties of the
>   		Source/Sink Data port N and are used by the bus to configure
>   		the Data Port N.
> +
> +What:		/sys/bus/soundwire/devices/sdw:.../status
> +Date:		September 2020
> +Contact:	Vinod Koul <vkoul@kernel.org>
> +Description:	Soundwire Slave status
> +		This reports the current status of the slave, e.g if it is
> +		UNATTACHED, Attached, Alert or Reserved
> diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c
> index f510071b0add..3d2cc7612d0d 100644
> --- a/drivers/soundwire/sysfs_slave.c
> +++ b/drivers/soundwire/sysfs_slave.c
> @@ -97,8 +97,25 @@ static ssize_t modalias_show(struct device *dev,
>   }
>   static DEVICE_ATTR_RO(modalias);
>   
> +static const char *const slave_status[SDW_SLAVE_MAX] = {
> +	[SDW_SLAVE_UNATTACHED] =  "UNATTACHED",
> +	[SDW_SLAVE_ATTACHED] = "Attached",
> +	[SDW_SLAVE_ALERT] = "Alert",
> +	[SDW_SLAVE_RESERVED] = "Reserved",
> +};
> +
> +static ssize_t status_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct sdw_slave *slave = dev_to_sdw_dev(dev);
> +
> +	return sprintf(buf, "%s\n", slave_status[slave->status]);
> +}
> +static DEVICE_ATTR_RO(status);
> +
>   static struct attribute *slave_attrs[] = {
>   	&dev_attr_modalias.attr,
> +	&dev_attr_status.attr,
>   	NULL,
>   };
>   ATTRIBUTE_GROUPS(slave);
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index 790823d2d33b..e8c9c20d38b0 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -73,12 +73,14 @@ enum {
>    * @SDW_SLAVE_ATTACHED: Slave is attached with bus.
>    * @SDW_SLAVE_ALERT: Some alert condition on the Slave
>    * @SDW_SLAVE_RESERVED: Reserved for future use
> + * @SDW_SLAVE_MAX: Last status value
>    */
>   enum sdw_slave_status {
>   	SDW_SLAVE_UNATTACHED = 0,
>   	SDW_SLAVE_ATTACHED = 1,
>   	SDW_SLAVE_ALERT = 2,
>   	SDW_SLAVE_RESERVED = 3,
> +	SDW_SLAVE_MAX,
>   };
>   
>   /**
>