Re: [PATCH v4 16/16] ACPI / DSD: Document references, ports and endpoints

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux