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 > > > 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) } -- Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx -- 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