| Message ID | 1410256619-3213-5-git-send-email-ankit.jindal@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 09, 2014 at 10:56:58AM +0100, Ankit Jindal wrote: > This patch adds device tree binding documentation for > Xgene QMTM UIO driver. > > Signed-off-by: Ankit Jindal <ankit.jindal@linaro.org> > Signed-off-by: Tushar Jagad <tushar.jagad@linaro.org> > --- > .../devicetree/bindings/uio/uio_xgene_qmtm.txt | 45 ++++++++++++++++++++ > 1 file changed, 45 insertions(+) > create mode 100644 Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt > > diff --git a/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt b/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt > new file mode 100644 > index 0000000..b71831b > --- /dev/null > +++ b/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt > @@ -0,0 +1,45 @@ > +APM X-Gene QMTM UIO nodes > + > +QMTM UIO nodes are defined for user space access to on-chip QMTM device > +on APM X-Gene SOC using UIO framework. > + Userspace vs kernel space has precisely _nothing_ to do with a HW description. This doesn't describe at all what the device is (e.g. what does QMTM stand for, what is it used for?). NAK. Please ensure you Cc the devicetree list (devicetree@vger.kernel.org) in future. > +Required properties: > +- compatible: Should be "apm,xgene-qmtm-uio" This should definitely not have "uio" in the name. > +- reg: Address and length of the register set for the device. It contains the > + information of registers in the same order as described by reg-names. > +- reg-names: Should contain the register set names > + - "csr": QMTM control and status register address space. > + - "fabric": QMTM memory mapped access to queue states. These look ok, I guess. > + - "qpool": Memory location for creating QMTM queues. This could be some > + SRAM or reserved portion of RAM. It is expected that size and location > + of qpool memory will be configurable via bootloader. I don't follow why this should be described in a reg entry. This is not a property of the device; this is a separate resource being assigned to the device. Why can the kernel not allocate this dynamically? If you need a specific fixed pool, use the reserved-memory bindings. > +- clocks: Reference to the clock entry. There is only one clock input to the IP block? > +- num_queues: Number of queues under this QMTM device. s/_/-/, property names should use '-' rather than '_'. > +- devid: QMTM identification number for the system having multiple QMTM devices What exactly is this used for? Why is the reg entry not sufficient? > +Optional properties: > +- status: Should be "ok" or "disabled" for enabled/disabled. Default is "ok". This is so standard I don't see the point in documenting it. Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9 September 2014 16:23, Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, Sep 09, 2014 at 10:56:58AM +0100, Ankit Jindal wrote: >> This patch adds device tree binding documentation for >> Xgene QMTM UIO driver. >> >> Signed-off-by: Ankit Jindal <ankit.jindal@linaro.org> >> Signed-off-by: Tushar Jagad <tushar.jagad@linaro.org> >> --- >> .../devicetree/bindings/uio/uio_xgene_qmtm.txt | 45 ++++++++++++++++++++ >> 1 file changed, 45 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt >> >> diff --git a/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt b/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt >> new file mode 100644 >> index 0000000..b71831b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt >> @@ -0,0 +1,45 @@ >> +APM X-Gene QMTM UIO nodes >> + >> +QMTM UIO nodes are defined for user space access to on-chip QMTM device >> +on APM X-Gene SOC using UIO framework. >> + > > Userspace vs kernel space has precisely _nothing_ to do with a HW > description. > > This doesn't describe at all what the device is (e.g. what does QMTM > stand for, what is it used for?). > > NAK. QMTM stands for queue manager and traffic manager. It is device managing the hardware queues. It also implements QoS among hardware queues hence term "traffic" manager is present in its name. Sure, I will add more info about the device in the next revision. The primary purpose of QMTM UIO driver is to expose QMTM HW to user space so that frameworks like ODP (OpenDataPlane) can use this from user space. > > Please ensure you Cc the devicetree list (devicetree@vger.kernel.org) in > future. Sure, will do. > >> +Required properties: >> +- compatible: Should be "apm,xgene-qmtm-uio" > > This should definitely not have "uio" in the name. I added "uio" in compatible string because in future APM might add separate driver for accessing QMTM from kernel space. Can you suggest better compatible strings for this UIO driver? > >> +- reg: Address and length of the register set for the device. It contains the >> + information of registers in the same order as described by reg-names. >> +- reg-names: Should contain the register set names >> + - "csr": QMTM control and status register address space. >> + - "fabric": QMTM memory mapped access to queue states. > > These look ok, I guess. > >> + - "qpool": Memory location for creating QMTM queues. This could be some >> + SRAM or reserved portion of RAM. It is expected that size and location >> + of qpool memory will be configurable via bootloader. > > I don't follow why this should be described in a reg entry. This is not > a property of the device; this is a separate resource being assigned to > the device. Ok, I will remove it from reg and add "qpool-addr" and "qpool-size" attributes to point the location of qpool. > > Why can the kernel not allocate this dynamically? For faster access, this can be on-chip memory as well. > > If you need a specific fixed pool, use the reserved-memory bindings. reserved-memory only apply to RAM, it does not apply to on-chip SRAMs. Right ? > >> +- clocks: Reference to the clock entry. > > There is only one clock input to the IP block? Yes. > >> +- num_queues: Number of queues under this QMTM device. > > s/_/-/, property names should use '-' rather than '_'. Sure, will rename it in next revision. > >> +- devid: QMTM identification number for the system having multiple QMTM devices > > What exactly is this used for? Why is the reg entry not sufficient? When there are multiple QMTM devices, the devid is used to form a unique id (a tuple of queue number and device id) for the queues belonging to this device. Unfortunately, we don't have any register providing the devid of the device. > >> +Optional properties: >> +- status: Should be "ok" or "disabled" for enabled/disabled. Default is "ok". > > This is so standard I don't see the point in documenting it. Ok, I will remove it in next revision. > > Mark. Thanks, Ankit -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt b/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt new file mode 100644 index 0000000..b71831b --- /dev/null +++ b/Documentation/devicetree/bindings/uio/uio_xgene_qmtm.txt @@ -0,0 +1,45 @@ +APM X-Gene QMTM UIO nodes + +QMTM UIO nodes are defined for user space access to on-chip QMTM device +on APM X-Gene SOC using UIO framework. + +Required properties: +- compatible: Should be "apm,xgene-qmtm-uio" +- reg: Address and length of the register set for the device. It contains the + information of registers in the same order as described by reg-names. +- reg-names: Should contain the register set names + - "csr": QMTM control and status register address space. + - "fabric": QMTM memory mapped access to queue states. + - "qpool": Memory location for creating QMTM queues. This could be some + SRAM or reserved portion of RAM. It is expected that size and location + of qpool memory will be configurable via bootloader. +- clocks: Reference to the clock entry. +- num_queues: Number of queues under this QMTM device. +- devid: QMTM identification number for the system having multiple QMTM devices + +Optional properties: +- status: Should be "ok" or "disabled" for enabled/disabled. Default is "ok". + +Example: + qmtm1clk: qmtmclk@1f20c000 { + compatible = "apm,xgene-device-clock"; + clock-output-names = "qmtm1clk"; + status = "ok"; + }; + + qmtm1_uio: qmtm_uio@1f200000 { + compatible = "apm,xgene-qmtm-uio"; + status = "disabled"; + reg = <0x0 0x1f200000 0x0 0x10000 + 0x0 0x1b000000 0x0 0x400000 + 0x0 0x00000000 0x0 0x0>;/* filled by bootloader */ + reg-names = "csr", "fabric", "qpool"; + clocks = <&qmtm1clk 0>; + num_queues = <0x400>; + devid = <1>; + }; + + /* Board-specific peripheral configurations */ + &qmtm1_uio { + status = "ok"; + };