Re: [PATCH v2 2/2] soundwire: sysfs: add slave status and device number before probe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






   * Base file is device
   *	|---- modalias
+ *	|---- dev-status
+ *		|---- status
+ *		|---- device_number

Any reason why we want this under dev-status.

Both the status and device_number belong to the device, so we can
put them under device and use device properties

We already use directories for device-level and port-level properties, I just thought it be cleaner to continue this model. We might also expand the information later on, e.g. provide interrupt status.

I don't mind if we remove the directory and move everything up one level, but it wouldn't be consistent with the previous work.

+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");

Do we really want N/A here, 0 should imply UNATTACHED and then the
status_show would tell UNATTACHED.

Actually no. If you look at the standard, 'Unattached' is an 'internal state of a Slave that indicates that it is not synchronized with to the Frame boundaries within the Bitstream'. A Slave device can only become attached and report it's presence as Device0 in a PING frame once it's ATTACHED - which in turn means the device has been able to sync for 15 frames. A device number of zero means the device is able to respond to command but has not yet been enumerated, or was enumerated previously but lost sync or went through a reset sequence and reattached. A device number of zero does not mean the device is unattached, the logic is as follow:

Attached -> Device 0 or 1..11
Unattached -> No concept of device number (or not an observable value).

We should not overload what 'Device0' means and instead follow the standard to the letter. We also don't want the attribute to come and go dynamically, so N/A (Not Applicable) is IMHO the only way to convey this meaning.

Does this help?



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux