Hi Matt,
On 13/06/18 16:47, Matt Sealey wrote:
Hi Suzuki,
Why not use “unit”?
I believe we had this discussion years ago about numbering serial ports
and sdhci (i.e. how do you know it’s UART0 or UART1 from just the address?
Some SoC’s don’t address sequentially *or* in a forward direction) - I
believe it’s not exactly codified in ePAPR, not am I sure where it may be
otherwise, but it exists.
We have different situation here. We need to know *the port number* as
understood by the hardware, so that we can enable *the specific* port for
a given path.
For the purposes of abstraction, each port will have the property of having
a node which is pointed to by other nodes, and in the case of a true ATB
endpoint, no other nodes behind it.
It doesn't matter what the HW numbers it as as long as the driver can derive
it from whatever you put in the DT. So a funnel (which is ~8 ports muxed into
one output):
f1p0: port {
unit = <0>;
endpoint = <&f1out>;
};
f1p1: port {
unit = <4>;
endpoint = <&f1out>;
};
f1out: port {
endpoint = <&etf1>;
};
"unit" here is specific to the driver's understanding of ports within it's
I may be missing, but is "unit" something that already exists and used by
DT bindings already ? Or is this something new that we are proposing ?
own cycle of the graph. For a replicator you can invert the logic - input
ports don't need a unit, but the two outputs are filtered in CoreSight not
I would prefer to make the new property mandatory for all the ports to avoid
a potential problem in the future.
How do you represent a TMC-ETF which has one input and one output connection ?
Also what happens if we ever get a component which has m-to-n connections ?
by leg but by transiting ATB ID in groups of 16 IDs. In that case maybe
you would want to describe all 8 possible units on each leg with the first
ID it would filter? Or just list tuples of filter IDs <id, first, last>
I am failing to follow the ATB ID group description above. As per the TRM,
e.g, replicator filters the "trace stream" based on the "trace ID", which I
believe can be programmed via IDFILTER<n> register. So why would we need that
to be part of the DT ?
Who cares, really, as long as the driver knows what it means.
You don't need to namespace every property.
As I mentioned above, we need the hardware numbers to enable the
"specific" port.
Okay and how is this not able to be prescribed in a binding for "arm,coresight-funnel"
that:
"input ports are numbered from 0 to N where N is the maximum input port
number. This number is identified with the "unit" property, which directly
corresponds to the bit position in the funnel Ctrl_Reg register, and the
bit position multiplied by 3 for each 3-bit priority in the funnel
Priority_Ctrl_Reg, with N having a maximum of the defined register bitfield
DEVID[PORTCOUNT], minus one, for that component"
The description looks over complicated to me at least, even after having known
bit of the programming interface of the components. I would prefer staying
closer to the terms used in the TRM ("slave/master" interfaces) and make it
easier for people to write the DT.
Or a replicator:
"output ports are numbered per the CoreSight ATB Replicator specification,
unit corresponding to the IDFILTERn register controlling ID filters for
that leg, with a maximum of the defined register bitfield DEVID[PORTNUM],
minus one"
One could clarify it, even, with labels for readability ("label" definitely
is a well defined if also completely arbitrary property).
..
static void funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
{
u32 functl;
CS_UNLOCK(drvdata->base);
functl = readl_relaxed(drvdata->base + FUNNEL_FUNCTL);
functl &= ~FUNNEL_HOLDTIME_MASK;
functl |= FUNNEL_HOLDTIME;
functl |= (1 << port);
writel_relaxed(functl, drvdata->base + FUNNEL_FUNCTL);
writel_relaxed(drvdata->priority, drvdata->base + FUNNEL_PRICTL);
CS_LOCK(drvdata->base);
}
No we don't need to parse it in both ways, up and down. Btw, the trace
paths are not statically created. They are done at runtime, as configured
by the user.
You do realize this isn't how the hardware works, correct?
The "trace paths" mentioned above were indeed the software path, which
was constructed at runtime. The graph connections are indeed a one time
parsing at probe time and as you said they don't change. And by configuring,
I mean selecting the "source" and the "sink".
Trace paths are fixed, they may diverge with different configurations, but
the full CoreSight topology (all funnels, replicators and intermediary
Components) is entirely unchangeable.
The DT should provide the information to provide a reference acyclic directed
graph of the entire topology (or entirely reasonably programmable topology where
at all possible) - if a user wants to trace from ETM_0 then they only
have particular paths to particular sinks, for instance ETM_0 and ETF_0
may be on their own path, so you cannot just "configure as a user"
a path from ETM_1 to ETF_0 since there isn't one.
Walking said graphs with the knowledge that CoreSight specifically disallows
loopbacks in ATB topology is basic computer science problem - literally a
matter of topological sorting. But let's build a graph once and traverse it -
don't build the graph partially each time or try and build it to cross-check
every time. The paths are wires in the design, lets not fake to the user
that there is any configurability in that or try and encode that in the
DT.
Sorry for the confusion, as explained above, it is indeed a one time pass.
Coming back to your suggestion of "unit", what does it imply ?
Whatever the driver likes. For uart and mmc, it was just a spurious number
but it could be applied as the end of, say, ttyS<N> or mmcblk<N>p3 or used
in any other driver-specific manner. The number you put in is up to you,
but the valid numbers would be in the binding for that particular device.
Its too generic a term for something as concrete as a port number.
Is it?
Why would you need a whole other property type to encode a u32 that
describes an arbitrary number specific to that hardware device?
So, if the suggestion is to use an existing property "unit", I am fine
with it, if people agree to it.
Thanks for the comments.
Cheers,
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
