All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sanyog Kale <sanyog.r.kale@intel.com>,
	Vinod Koul <vkoul@kernel.org>,
	Bard Liao <yung-chuan.liao@linux.intel.com>,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] soundwire: sysfs: cleanup the logic for creating the dp0 sysfs attributes
Date: Fri, 29 Jul 2022 11:46:32 -0500	[thread overview]
Message-ID: <701aa1ba-9b25-51eb-8bd7-2389b501d79c@linux.intel.com> (raw)
In-Reply-To: <YuQMYRYFo9gTk1yL@kroah.com>


>>>>> That should be fine, tools should just be looking for the attributes,
>>>>> not the existance of a directory, right?
>>>>
>>>> The idea what that we would only expose ports that actually exist.
>>>> That's helpful information anyone with a basic knowledge of the
>>>> SoundWire specification would understand.
>>>
>>> Is "dp0" a port?  If so, why isn't it a real device?
>>
>> The SoundWire spec defines the concept of 'data port'. The valid ranges
>> are 1..14, but in all existing devices the number of data ports is way
>> smaller, typically 2 to 4. Data ports (DPn) are source or sink, and
>> there's no firm rule that data ports needs to be contiguous.
>>
>> DP0 is a 'special case' where the data transport is used for control
>> information, e.g. programming large set of registers or firmware
>> download. DP0 is completely optional in hardware, and not handled in
>> Linux for now.
>>
>> DP0 and DPn expose low-level transport registers, which define how the
>> contents of a FIFO will be written or read from the bus. Think of it as
>> a generalization of the concept of TDM slots, where instead of having a
>> fixed slot per frame the slot position/repetition/runlength can be
>> programmed.
>>
>> The data ports could be as simple as 1-bit PDM, or support 8ch PCM
>> 24-bits. That's the sort of information reported in attributes.
> 
> Why not make them a real device like we do for USB endpoints?

I don't see what adding another layer of hierarchy would bring. In their
simplest configuration, there are 6 registers 8-bit exposed. And the
port registers, when present, are accessed with a plain vanilla offset.

> What uses these sysfs files today that would be confused about an empty
> directory?

That's a good question. I am not aware of any tools making use of those
attributes. To a large degree, they are helpful only for debug and
support, all these read-only attributes could be moved to debugfs. That
could be a way to simplify everyone's life....

WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Vinod Koul <vkoul@kernel.org>,
	alsa-devel@alsa-project.org,
	Bard Liao <yung-chuan.liao@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	Sanyog Kale <sanyog.r.kale@intel.com>
Subject: Re: [PATCH 2/5] soundwire: sysfs: cleanup the logic for creating the dp0 sysfs attributes
Date: Fri, 29 Jul 2022 11:46:32 -0500	[thread overview]
Message-ID: <701aa1ba-9b25-51eb-8bd7-2389b501d79c@linux.intel.com> (raw)
In-Reply-To: <YuQMYRYFo9gTk1yL@kroah.com>


>>>>> That should be fine, tools should just be looking for the attributes,
>>>>> not the existance of a directory, right?
>>>>
>>>> The idea what that we would only expose ports that actually exist.
>>>> That's helpful information anyone with a basic knowledge of the
>>>> SoundWire specification would understand.
>>>
>>> Is "dp0" a port?  If so, why isn't it a real device?
>>
>> The SoundWire spec defines the concept of 'data port'. The valid ranges
>> are 1..14, but in all existing devices the number of data ports is way
>> smaller, typically 2 to 4. Data ports (DPn) are source or sink, and
>> there's no firm rule that data ports needs to be contiguous.
>>
>> DP0 is a 'special case' where the data transport is used for control
>> information, e.g. programming large set of registers or firmware
>> download. DP0 is completely optional in hardware, and not handled in
>> Linux for now.
>>
>> DP0 and DPn expose low-level transport registers, which define how the
>> contents of a FIFO will be written or read from the bus. Think of it as
>> a generalization of the concept of TDM slots, where instead of having a
>> fixed slot per frame the slot position/repetition/runlength can be
>> programmed.
>>
>> The data ports could be as simple as 1-bit PDM, or support 8ch PCM
>> 24-bits. That's the sort of information reported in attributes.
> 
> Why not make them a real device like we do for USB endpoints?

I don't see what adding another layer of hierarchy would bring. In their
simplest configuration, there are 6 registers 8-bit exposed. And the
port registers, when present, are accessed with a plain vanilla offset.

> What uses these sysfs files today that would be confused about an empty
> directory?

That's a good question. I am not aware of any tools making use of those
attributes. To a large degree, they are helpful only for debug and
support, all these read-only attributes could be moved to debugfs. That
could be a way to simplify everyone's life....

  reply	other threads:[~2022-07-29 16:47 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-29 13:50 [PATCH 1/5] soundwire: sysfs: move sdw_slave_dev_attr_group into the existing list of groups Greg Kroah-Hartman
2022-07-29 13:50 ` Greg Kroah-Hartman
2022-07-29 13:50 ` [PATCH 2/5] soundwire: sysfs: cleanup the logic for creating the dp0 sysfs attributes Greg Kroah-Hartman
2022-07-29 13:50   ` Greg Kroah-Hartman
2022-07-29 14:46   ` Pierre-Louis Bossart
2022-07-29 14:46     ` Pierre-Louis Bossart
2022-07-29 14:52     ` Greg Kroah-Hartman
2022-07-29 14:52       ` Greg Kroah-Hartman
2022-07-29 14:57       ` Pierre-Louis Bossart
2022-07-29 14:57         ` Pierre-Louis Bossart
2022-07-29 15:03         ` Greg Kroah-Hartman
2022-07-29 15:03           ` Greg Kroah-Hartman
2022-07-29 15:52           ` Pierre-Louis Bossart
2022-07-29 15:52             ` Pierre-Louis Bossart
2022-07-29 16:35             ` Greg Kroah-Hartman
2022-07-29 16:35               ` Greg Kroah-Hartman
2022-07-29 16:46               ` Pierre-Louis Bossart [this message]
2022-07-29 16:46                 ` Pierre-Louis Bossart
2022-07-29 17:15                 ` Greg Kroah-Hartman
2022-07-29 17:15                   ` Greg Kroah-Hartman
2022-07-29 17:25                   ` Pierre-Louis Bossart
2022-07-29 17:25                     ` Pierre-Louis Bossart
2022-08-24 13:55                 ` Greg Kroah-Hartman
2022-08-24 13:55                   ` Greg Kroah-Hartman
2022-08-03 11:30           ` Vinod Koul
2022-08-03 11:30             ` Vinod Koul
2022-07-29 13:50 ` [PATCH 3/5] soundwire: sysfs: have the driver core handle the creation of the device groups Greg Kroah-Hartman
2022-07-29 13:50   ` Greg Kroah-Hartman
2022-07-29 14:12   ` Pierre-Louis Bossart
2022-07-29 14:12     ` Pierre-Louis Bossart
2022-07-29 14:19     ` Greg Kroah-Hartman
2022-07-29 14:19       ` Greg Kroah-Hartman
2022-07-29 13:50 ` [PATCH 4/5] soundwire: sysfs: remove sdw_slave_sysfs_init() Greg Kroah-Hartman
2022-07-29 13:50   ` Greg Kroah-Hartman
2022-07-29 15:00   ` Pierre-Louis Bossart
2022-07-29 15:13     ` Greg Kroah-Hartman
2022-07-29 15:13       ` Greg Kroah-Hartman
2022-08-03 11:33       ` Vinod Koul
2022-08-03 11:33         ` Vinod Koul
2022-07-29 13:50 ` [PATCH 5/5] soundwire: sysfs: remove unneeded ATTRIBUTE_GROUPS() comments Greg Kroah-Hartman
2022-07-29 13:50   ` Greg Kroah-Hartman
2022-08-23 16:00 ` [PATCH 1/5] soundwire: sysfs: move sdw_slave_dev_attr_group into the existing list of groups Vinod Koul
2022-08-23 16:00   ` Vinod Koul
2022-08-24  6:18   ` Greg Kroah-Hartman
2022-08-24  6:18     ` Greg Kroah-Hartman
2022-08-24 14:04   ` Greg Kroah-Hartman
2022-08-24 14:04     ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=701aa1ba-9b25-51eb-8bd7-2389b501d79c@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sanyog.r.kale@intel.com \
    --cc=vkoul@kernel.org \
    --cc=yung-chuan.liao@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.