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

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

>>    * 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?