Hi,
On Mon, Mar 23, 2020 at 02:36:23PM -0500, Pierre-Louis Bossart wrote:
> Add attribute and show the components string. This is useful to see
> what is provided to userspace and e.g. used by UCM to understand the
> card configuration:
>
> root@plb:~# more /sys/class/sound/card0/components
> HDA:8086280b,80860101,00100000 cfg-spk:2 hs:rt711 spk:rt1308 mic:rt715
>
> Note that the style uses what's recommended by checkpatch.pl and is
> slightly different from other sysfs attributes.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> ---
> sound/core/init.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
I have a concern about this patch in a point of userspace interface.
The 'component' field of 'struct snd_ctl_card_info' is just defined to
have strings with space-separated chunks, and its actual value is not
so fine-documented, thus it's largely different depending of developers
of each driver.
$ cat include/uapi/sound/asound.h
...
941 struct snd_ctl_card_info {
...
950 unsigned char components[128]; /* card components / fine identification, delimited with one space (AC97 etc..) */
951 };
On the other hand, the node of sysfs is quite common in Linux because
it's tightly coupled to kernel objects. Let you see files under
'Documentation/ABI/'. We can find efforts to maintain sysfs node so
safe and stable. Due to this reason, it's better to take more care when
adding new node.
Would I request you the reason to add this node and the reason that
current ALSA control interface doesn't satisfy your requirement?
> diff --git a/sound/core/init.c b/sound/core/init.c
> index b02a99766351..decaf944a8ad 100644
> --- a/sound/core/init.c
> +++ b/sound/core/init.c
> @@ -695,9 +695,21 @@ card_number_show_attr(struct device *dev,
>
> static DEVICE_ATTR(number, 0444, card_number_show_attr, NULL);
>
> +static ssize_t
> +components_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct snd_card *card = container_of(dev, struct snd_card, card_dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%s\n", card->components);
> +}
> +
> +static DEVICE_ATTR_RO(components);
> +
> static struct attribute *card_dev_attrs[] = {
> &dev_attr_id.attr,
> &dev_attr_number.attr,
> + &dev_attr_components.attr,
> NULL
> };
>
> --
> 2.20.1
Regards
Takashi Sakamoto
