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? > 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. And analogously for the "enpoint" property value. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html