On Fri, Jan 15, 2016 at 03:16:38PM +0000, Mark Rutland wrote: > On Mon, Jan 11, 2016 at 09:45:42AM -0500, Sinan Kaya wrote: > > Add documentation for the Qualcomm Technologies HIDMA driver. > > s/driver/binding/ > > > > Signed-off-by: Sinan Kaya <okaya@xxxxxxxxxxxxxx> > > Acked-by: Rob Herring <robh@xxxxxxxxxx> Further to my reply below, I'm generally uncomfortable with some properties (max-* need a better description if they are a HW requirement, and probably should not be present otherwise). I'm also concerned that information necessary for the advertised use-case of the device (e.g. IOMMUs) is missing [1], and we're missing parts of the story necessary to review this for correctness. Until that's settled, I don't think this is ready yet, and I don't think this should be picked up. I'm sorry to say that at this stage, as I realise that may sound obstructive. That's not my intention, and I hope we can figure out those details quickly. Thanks, Mark. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/399850.html > > --- > > .../devicetree/bindings/dma/qcom_hidma_mgmt.txt | 79 ++++++++++++++++++++++ > > 1 file changed, 79 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt > > > > diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt > > new file mode 100644 > > index 0000000..0e6ed1f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt > > @@ -0,0 +1,79 @@ > > +Qualcomm Technologies HIDMA Management interface > > + > > +Qualcomm Technologies HIDMA is a high speed DMA device. It only supports > > +memcpy and memset capabilities. It has been designed for virtualized > > +environments. > > + > > +Each HIDMA HW instance consists of multiple DMA channels. These channels > > +share the same bandwidth. The bandwidth utilization can be parititioned > > +among channels based on the priority and weight assignments. > > + > > +There are only two priority levels and 15 weigh assignments possible. > > + > > +Other parameters here determine how much of the system bus this HIDMA > > +instance can use like maximum read/write request and and number of bytes to > > +read/write in a single burst. > > + > > +Main node required properties: > > +- compatible: "qcom,hidma-mgmt-1.0"; > > +- reg: Address range for DMA device > > +- dma-channels: Number of channels supported by this DMA controller. > > +- max-write-burst-bytes: Maximum write burst in bytes. A memcpy requested is > > + fragmented to multiples of this amount. > > +- max-read-burst-bytes: Maximum read burst in bytes. A memcpy request is > > + fragmented to multiples of this amount. > > +- max-write-transactions: Maximum write transactions to perform in a burst > > +- max-read-transactions: Maximum read transactions to perform in a burst > > Just to check, where do these max-* values come from? > > Are they some correctness requirement of the bus this is attached to? > > Are they tuning values? > > The latter doesn't really belong in the DT. Given they're writeable from > the driver, it seems like that's what they are... > > > +- channel-reset-timeout-cycles: Channel reset timeout in cycles for this SOC. > > I'm not sure what this means. Could you elaborate on this is? > > > + > > +Sub-nodes: > > + > > +HIDMA has one or more DMA channels that are used to move data from one > > +memory location to another. > > + > > +Each DMA channel is described as a sub-node under the management object. > > +When a transfer channel is given to the guest operating system, only the channel > > +object is created. The drivers have support for both flat and hierarchical > > +configuration. > > Don't mention drivers here. > > All you need to state is that when the OS is not in control of the > management interface (i.e. it's a guest), the channel nodes appear on > their own, not under a management node. > > Other than the above questions, this looks ok to me. > > Thanks, > Mark. > > > + > > +Required properties: > > +- compatible: must contain "qcom,hidma-1.0" > > +- reg: Addresses for the transfer and event channel > > +- interrupts: Should contain the event interrupt > > +- desc-count: Number of asynchronous requests this channel can handle > > +- channel-index: The HW event channel completions will be delivered. > > + > > +Example: > > + > > +Hypervisor OS configuration: > > + > > + hidma-mgmt@f9984000 = { > > + compatible = "qcom,hidma-mgmt-1.0"; > > + reg = <0xf9984000 0x15000>; > > + dma-channels = <6>; > > + max-write-burst-bytes = <1024>; > > + max-read-burst-bytes = <1024>; > > + max-write-transactions = <31>; > > + max-read-transactions = <31>; > > + channel-reset-timeout-cycles = <0x500>; > > + > > + hidma_24: dma-controller@0x5c050000 { > > + compatible = "qcom,hidma-1.0"; > > + reg = <0 0x5c050000 0x0 0x1000>, > > + <0 0x5c0b0000 0x0 0x1000>; > > + interrupts = <0 389 0>; > > + desc-count = <10>; > > + channel-index = <4>; > > + }; > > + }; > > + > > +Guest OS configuration: > > + > > + hidma_24: dma-controller@0x5c050000 { > > + compatible = "qcom,hidma-1.0"; > > + reg = <0 0x5c050000 0x0 0x1000>, > > + <0 0x5c0b0000 0x0 0x1000>; > > + interrupts = <0 389 0>; > > + desc-count = <10>; > > + channel-index = <4>; > > + }; > > -- > > Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project > > > -- > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
