On Wednesday, March 15, 2017 01:45:48 PM Sakari Ailus wrote:
> On 03/15/17 13:28, Rafael J. Wysocki wrote:
> > On Wednesday, March 15, 2017 11:33:35 AM Sakari Ailus wrote:
> >> On 03/15/17 10:23, Sakari Ailus wrote:
> >>> On Wed, Mar 15, 2017 at 12:13:45AM +0100, Rafael J. Wysocki wrote:
> >>>> On Tue, Mar 14, 2017 at 11:53 PM, Sakari Ailus
> >>>> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >>>>> Rafael J. Wysocki wrote:
> >>>>>>
> >>>>>> On Tue, Mar 14, 2017 at 10:16 PM, Sakari Ailus
> >>>>>> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >>>>>>>
> >>>>>>> Rafael J. Wysocki wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Tue, Mar 14, 2017 at 6:54 PM, Sakari Ailus
> >>>>>>>> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Hi Rafael,
> >>>>>>>>>
> >>>>>>>>> Rafael J. Wysocki wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On Tue, Mar 14, 2017 at 9:09 AM, Sakari Ailus
> >>>>>>>>>> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On 03/14/17 10:08, Sakari Ailus wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> How about this instead:
> >>>>>>>>>>>>
> >>>>>>>>>>>> All port nodes are located under the device's "_DSD" node in the
> >>>>>>>>>>>> hierarchical data extension tree. The property extension related to
> >>>>>>>>>>>> each port node must contain the key "port" and an integer value
> >>>>>>>>>>>> which
> >>>>>>>>>>>> is the number of the port.
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> So with matching strings instead of indices, this will change, too...
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> It doesn't have to AFAICS, but the number is just redundant IMO. You
> >>>>>>>>>> only need a boolean property saying "this is a port", so you know that
> >>>>>>>>>> you should expect a list of endpoints in that object.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> No, it's not redundant. It's the number of the physical port in the
> >>>>>>>>> device
> >>>>>>>>> --- this is how the driver gets to know where the connection has been
> >>>>>>>>> made.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> OK, but what exactly do you mean by "physical port"?
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> The device (or an IP block) has physical interfaces to the world outside.
> >>>>>>> There could be just one, but there may be more. For an ISP, there could
> >>>>>>> be
> >>>>>>> e.g. four CSI-2 receivers to each of which you could connect a camera
> >>>>>>> sensor. So for an ISP device, that number tells which of the receivers a
> >>>>>>> given sensor is connected to.
> >>>>>>>
> >>>>>>> The mapping between this number and what the hardware datasheet refers to
> >>>>>>> needs to be documented per device.
> >>>>>>
> >>>>>>
> >>>>>> OK, so the number actually is an arbitrary piece of data associated
> >>>>>> with the key "port" and the interpretation of that piece of data
> >>>>>> depends on whoever asks for that value.
> >>>>>>
> >>>>>> IOW, the core doesn't care.
> >>>>>>
> >>>>>> With all due respect to whoever invented this on the DT side, this is
> >>>>>> just bad design to me, because it causes the "port" property to serve
> >>>>>> two different purposes at the same time. First, it tells the core
> >>>>>> that this object is a port. Second, it is expected to provide a piece
> >>>>>> of data of unspecified interpretation to somebody. Which means that
> >>>>>> the "port" property is both general and device-specific at the same
> >>>>>> time and the sanity of that is quite questionable IMO.
> >>>>>
> >>>>>
> >>>>> DT uses a node called either "port" or "ports" to store the port nodes. The
> >>>>> reg property tells the number of the port (see
> >>>>> Documentation/devicetree/bindings/graph.txt). I'm no DT expert, but my
> >>>>> understanding is that the node namespace is different from the property
> >>>>> namespace.
> >>>>
> >>>> So on the DT side it actually looks OK to me.
> >>>>
> >>>> And the <reg> value is referred to as the port-endpoint identifier, so
> >>>> I guess this is used for referring to the port/endpoint instead of an
> >>>> index or the key value somewhere?
> >>>
> >>> The remote-endpoint uses phandles; they're a mechanism in DT to refer to
> >>> different nodes in the tree (DT does not differentiate between devices and
> >>> non-device nodes). There's a relatively good example here:
> >>>
> >>> arch/arm/boot/dts/omap3-n9.dts
> >
> > Yes, I figured that out. :-)
> >
> > Still, the <rev> property value is used for something somewhere in the code I gather.
> >
> >>>>
> >>>>> If you're concerned of possible double meanings, it's entirely possible to
> >>>>> put the port nodes under hierarchical data extension named e.g. "ports", and
> >>>>> document that this is what the node must be called (single port node could
> >>>>> be just called "port"). This way, it should be much more difficult to
> >>>>> interpret a non-port node as a port node --- roughly equivalent of the DT
> >>>>> ports node.
> >>>>>
> >>>>> The drawback with this change is that the size of the data structure in ASL
> >>>>> (and AML) will grow.
> >>>>
> >>>> The "ports" thing would only be useful if we had the other properties
> >>>> to put in there.
> >>>>
> >>>> So I guess we can specify that the "port" property value is the
> >>>> identifier of the port and then we will use this in the
> >>>> "remote-endpoint" property on the other end instead of an index.
> >>>
> >>> Makes sense.
> >>>
> >>>>
> >>>> And analogously for the "enpoint" property value.
> >>>
> >>> The endpoint hierarchical data extension node name? There is no endpoint
> >>> property being used by this version of the set anymore; I removed it as it
> >>> was redundant. However a human readable endpoint node name can be chosen
> >>> which that'd be quite practical to identify the endpoint.
> >>>
> >>> Then the remote-endpoint properties would be:
> >>>
> >>> Package () { device, port (integer), endpoint-node-name (string) }
> >>>
> >>
> >> Oh, well... the device is present there already, so the endpoint
> >> reference would well use an index pretty much equally well. Most of the
> >> time it'll be zero anyway. I.e.
> >>
> >> Package() { device, port number (integer), endpoint id (integer }
> >
> > Yes, I would do that, and actually not using the index for the endpoint too.
> >
> > Let ports and endpoints be symmetrical in that respect, that is a port is
> > required to have a { "port", id } property and an endpoind is required to
> > have a { "endpoint", id } property and let the ids be used in
> > "remote-endpoint" properties as per the above.
> >
> > Then, in each case, the id would be whatever the value of the <rev> property
> > on the DT side would be.
>
> DT graphs only have port numbers, the endpoints are not referred to by
> IDs --- just phandles. The closest equivalent we have in ACPI is a
> device reference as far as I can tell, and this is why the port number
> and some identifier for the endpoint is required.
>
> Adding a numeric ID for the endpoint is somewhat artificial. Most would
> just have zero there as there are commonly only a single endpoint in a
> port. And if there were more than one, one of the most sensible
> approaches to number them would be a monotonically incrementing number
> from zero --- which is the same than the index.
>
> So my view is that adding an endpoint property simply adds no value.
But endpoints can have a <rev> property in DT. What is it used for then?
Thanks,
Rafael
--
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
