linux-kernel - Re: [PATCH] soundwire: bus: add enumerated slave to device list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f3880470-cdc4-7b84-97a1-303f9a95d3f4@linux.intel.com>
Date:   Thu, 10 Sep 2020 09:02:36 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     yung-chuan.liao@...ux.intel.com, sanyog.r.kale@...el.com,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soundwire: bus: add enumerated slave to device list


>>> May be we could make the enumerated devices discovery bit more verbose!
>>
>> Maybe adding a device number sysfs entry would help, e.g. reporting
>> NotAttched or a value in [0,11] would tell you if the device is actually
>> present.
> 
> Agreed, I cooked this patch to report verbose device status, let me know
> if you are okay with this. I think this would be useful regardless of
> current discussion.
> 
> On Db845c I see:
> 
> root@...aro-alip:/sys/bus/soundwire/devices# cat sdw:0:217:2010:0:1/status
> Attached
> root@...aro-alip:/sys/bus/soundwire/devices# cat sdw:0:217:2010:0:2/status
> Attached

looks like we are all aligned on the idea, I have a similar patch to at 
https://github.com/thesofproject/linux/pull/2426

The difference is that the sysfs status and device_number is added even 
without a driver probe and when there's no firmware description. sysfs 
is currently only added after the driver probe, which wouldn't work for 
the case Srinivas was trying to deal with.

but the way you dealt the status below is better than the switch case I 
used, so will merge this into my code.

Srinivas' patch needs a fix for ACPI platforms, won't probe otherwise 
since we don't have an of_node. If that's alright with everyone I can 
submit a patchset that gathers the 3 contributions.

> 
> diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c
> index f510071b0add..3b2765f10024 100644
> --- a/drivers/soundwire/sysfs_slave.c
> +++ b/drivers/soundwire/sysfs_slave.c
> @@ -97,8 +97,27 @@ static ssize_t modalias_show(struct device *dev,
>   }
>   static DEVICE_ATTR_RO(modalias);
>   
> +#define SDW_SLAVE_MAX (SDW_SLAVE_RESERVED + 1)
> +
> +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);
>