On Thu, 24 May 2018 09:32:48 -0600 Mathieu Poirier <[email protected]> wrote:
> On 23 May 2018 at 13:51, Kim Phillips <[email protected]> wrote: > > On Tue, 22 May 2018 11:31:40 -0600 > > Mathieu Poirier <[email protected]> wrote: > > > >> On Thu, May 17, 2018 at 08:20:19PM -0500, Kim Phillips wrote: > >> > A coresight topology doesn't need to include links, i.e., a source can > >> > be directly connected to a sink. As such, selecting and/or depending on > >> > LINKS_AND_SINKS is no longer needed. > >> > >> I'm good with this patch but now the help text for > >> CORESIGHT_LINKS_AND_SINKS no > >> longer match what the config does. I see two ways to fix this: > > > > This patch doesn't change what the config does, it just changes what > > other config options depend on it. > > > >> 1) Rework the help text. > > > > I don't see how, given the above. Here's the text: > > > > config CORESIGHT_LINKS_AND_SINKS > > bool "CoreSight Link and Sink drivers" > > help > > This enables support for CoreSight link and sink drivers that are > > responsible for transporting and collecting the trace data > > respectively. Link and sinks are dynamically aggregated with a > > trace > > entity at run time to form a complete trace path. > > > > What part of that becomes invalid with this patch? > > Looking at the new Kconfig, what sink component depend on > CORESIGHT_LINKS_AND_SINKS? How does that affect the description text? The description text doesn't insinuate any implicit dependencies or non-. > config CORESIGHT_LINKS Please, not another gratuitous config name change, I've already experienced usage regressions from the CORESIGHT_QCOM_REPLICATOR => CORESIGHT_DYNAMIC_REPLICATOR change: https://patchwork.kernel.org/patch/10206023/ > bool "CoreSight Link drivers" > help > This enables support for CoreSight link drivers that are > responsible > for transporting trace data from source to sink. Links are > dynamically > aggregated with other traces entities at run time to form a > complete trace > path. Oh, I see, so your point is that LINKS_AND_SINKS doesn't technically build any sink drivers? That's completely orthogonal to removing a dependency chain: that just tells me the name was a poor choice in the first place maybe? I don't see where the Makefile may have built a sink, but it may be before the move to drivers/hwtracing/coresight, or some other reorganization. > >> 2) Rework CORESIGHT_LINKS_AND_SINKS to be CORESIGHT_FUNNEL and move > >> coresight-replicator.o under CORESIGHT_DYNAMIC_REPLICATOR in the Makefile. > >> I > >> really liked your idea of making the replicator driver intelligent enough > >> to > >> deal with both DT and platform declaration, which merges two driver into > >> one. > >> > >> I'm obviously favouring the second option but recognise it doesn't have to > >> be > >> part of this patchet. So for this set please rework the help text for > >> CORESIGHT_LINKS_AND_SINKS. Once we've dealt with this topic we can > >> refactor the > >> replicator driver. > > > > I'd really like to just focus on getting CoreSight to load as modules, > > something for which this patch isn't technically required... > > The only thing I'm asking is that the config description and help text > reflect what the Makefile does. argh, wellll, it's a completely different change, and we're now completely off the modularization topic, and I'm uncomfortable doing reorgs on things I don't understand, renaming CONFIG_s, esp. when others such as the REPLICATOR, since as far as I know, that's also a link?? Kim
