soundwire: sysfs: add slave status and device number before probe

The MIPI DisCo device properties that are read by the driver from
platform firmware, or hard-coded in the driver, should only be
provided as sysfs entries when a driver probes successfully.

However the device status and device number is updated even when there
is no driver present, and hence can be updated when a Slave device is
detected on the bus without being described in platform firmware and
without any driver registered/probed.

As suggested by GregKH, the attribute group for Slave status and
device number is is added by default upon device registration.

Credits to Vinod Koul for the status_show() function, shared in a
separate patch and used as is here. The status table was modified to
remove an unnecessary enum and status_show() is handled in a different
group attribute than what was suggested by Vinod.

Tested-by: Srinivas Kandgatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Co-developed-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20200924194430.121058-3-pierre-louis.bossart@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

diff --git a/Documentation/ABI/testing/sysfs-bus-soundwire-slave b/Documentation/ABI/testing/sysfs-bus-soundwire-slave
index db4c9511d1aa9aacf1c2f9825f1e62337bdd136c..d324aa0b678f4d9fd1fb8c4d39bfeafafe73c247 100644 (file)
--- a/Documentation/ABI/testing/sysfs-bus-soundwire-slave
+++ b/Documentation/ABI/testing/sysfs-bus-soundwire-slave
@@ -1,3 +1,21 @@
+What:          /sys/bus/soundwire/devices/sdw:.../status
+               /sys/bus/soundwire/devices/sdw:.../device_number
+
+Date:          September 2020
+
+Contact:       Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
+               Bard Liao <yung-chuan.liao@linux.intel.com>
+               Vinod Koul <vkoul@kernel.org>
+
+Description:   SoundWire Slave status
+
+               These properties report the Slave status, e.g. if it
+               is UNATTACHED or not, and in the latter case show the
+               device_number. This status information is useful to
+               detect devices exposed by platform firmware but not
+               physically present on the bus, and conversely devices
+               not exposed in platform firmware but enumerated.
+
 What:          /sys/bus/soundwire/devices/sdw:.../dev-properties/mipi_revision
                /sys/bus/soundwire/devices/sdw:.../dev-properties/wake_capable
                /sys/bus/soundwire/devices/sdw:.../dev-properties/test_mode_capable

diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 19b012310c295836673d57614813438047835684..a08f4081c1c4810652cbebedf2460433645c8ad6 100644 (file)
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -6,6 +6,7 @@
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_type.h>
 #include "bus.h"
+#include "sysfs_local.h"
 
 static void sdw_slave_release(struct device *dev)
 {
@@ -51,6 +52,7 @@ int sdw_slave_add(struct sdw_bus *bus,
        slave->dev.bus = &sdw_bus_type;
        slave->dev.of_node = of_node_get(to_of_node(fwnode));
        slave->dev.type = &sdw_slave_type;
+       slave->dev.groups = sdw_slave_status_attr_groups;
        slave->bus = bus;
        slave->status = SDW_SLAVE_UNATTACHED;
        init_completion(&slave->enumeration_complete);

diff --git a/drivers/soundwire/sysfs_local.h b/drivers/soundwire/sysfs_local.h
index ff60adee3c414323137a8b73522d84b5e31808d0..7268bc24c5384f699c3b9bfab9b06a020051d813 100644 (file)
--- a/drivers/soundwire/sysfs_local.h
+++ b/drivers/soundwire/sysfs_local.h
@@ -8,6 +8,10 @@
  * SDW sysfs APIs -
  */
 
+/* basic attributes to report status of Slave (attachment, dev_num) */
+extern const struct attribute_group *sdw_slave_status_attr_groups[];
+
+/* additional device-managed properties reported after driver probe */
 int sdw_slave_sysfs_init(struct sdw_slave *slave);
 int sdw_slave_sysfs_dpn_init(struct sdw_slave *slave);
 

diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c
index f510071b0addca746607dcf243091deb65b0e659..b48b6617a3964115d41aa871b40097c446e83ff3 100644 (file)
--- a/drivers/soundwire/sysfs_slave.c
+++ b/drivers/soundwire/sysfs_slave.c
@@ -16,9 +16,13 @@
 
 /*
  * The sysfs for Slave reflects the MIPI description as given
- * in the MIPI DisCo spec
+ * in the MIPI DisCo spec.
+ * status and device_number come directly from the MIPI SoundWire
+ * 1.x specification.
  *
  * Base file is device
+ *     |---- status
+ *     |---- device_number
  *     |---- modalias
  *     |---- dev-properties
  *             |---- mipi_revision
@@ -212,3 +216,55 @@ int sdw_slave_sysfs_init(struct sdw_slave *slave)
 
        return 0;
 }
+
+/*
+ * the status is shown in capital letters for UNATTACHED and RESERVED
+ * on purpose, to highligh users to the fact that these status values
+ * are not expected.
+ */
+static const char *const slave_status[] = {
+       [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 ssize_t device_number_show(struct device *dev,
+                                 struct device_attribute *attr, char *buf)
+{
+       struct sdw_slave *slave = dev_to_sdw_dev(dev);
+
+       if (slave->status == SDW_SLAVE_UNATTACHED)
+               return sprintf(buf, "%s", "N/A");
+       else
+               return sprintf(buf, "%d", slave->dev_num);
+}
+static DEVICE_ATTR_RO(device_number);
+
+static struct attribute *slave_status_attrs[] = {
+       &dev_attr_status.attr,
+       &dev_attr_device_number.attr,
+       NULL,
+};
+
+/*
+ * we don't use ATTRIBUTES_GROUP here since the group is used in a
+ * separate file and can't be handled as a static.
+ */
+static const struct attribute_group sdw_slave_status_attr_group = {
+       .attrs  = slave_status_attrs,
+};
+
+const struct attribute_group *sdw_slave_status_attr_groups[] = {
+       &sdw_slave_status_attr_group,
+       NULL
+};