Messages in this thread | | | Date | Thu, 24 May 2018 18:30:27 -0500 | From | Kim Phillips <> | Subject | Re: [PATCH 1/6] coresight: remove CORESIGHT_LINKS_AND_SINKS dependencies and selections |
| |
On Thu, 24 May 2018 09:32:48 -0600 Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 23 May 2018 at 13:51, Kim Phillips <kim.phillips@arm.com> wrote: > > On Tue, 22 May 2018 11:31:40 -0600 > > Mathieu Poirier <mathieu.poirier@linaro.org> 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
| |