From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
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 19:15:24 +0200 [thread overview]
Message-ID: <YuQVrLEqPMu1V0zJ@kroah.com> (raw)
In-Reply-To: <701aa1ba-9b25-51eb-8bd7-2389b501d79c@linux.intel.com>
On Fri, Jul 29, 2022 at 11:46:32AM -0500, Pierre-Louis Bossart wrote:
>
> >>>>> 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.
Who uses these registers?
> > 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....
That would be much nicer, put it all in a single debugfs file and it
would be so simple.
What attributes could we do that for?
thanks,
greg k-h
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
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 19:15:24 +0200 [thread overview]
Message-ID: <YuQVrLEqPMu1V0zJ@kroah.com> (raw)
In-Reply-To: <701aa1ba-9b25-51eb-8bd7-2389b501d79c@linux.intel.com>
On Fri, Jul 29, 2022 at 11:46:32AM -0500, Pierre-Louis Bossart wrote:
>
> >>>>> 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.
Who uses these registers?
> > 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....
That would be much nicer, put it all in a single debugfs file and it
would be so simple.
What attributes could we do that for?
thanks,
greg k-h
next prev parent reply other threads:[~2022-07-29 17:16 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
2022-07-29 16:46 ` Pierre-Louis Bossart
2022-07-29 17:15 ` Greg Kroah-Hartman [this message]
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=YuQVrLEqPMu1V0zJ@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=alsa-devel@alsa-project.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--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.