On Mon, Mar 10, 2014 at 11:14:31PM +0000, Vince Bridgers wrote:
> This patch adds a bindings description for the Altera Triple Speed Ethernet
> (TSE) driver. The bindings support the legacy SGDMA soft IP as well as the
> preferred MSGDMA soft IP. The TSE can be configured and synthesized in soft
> logic using Altera's Quartus toolchain. Please consult the bindings document
> for supported options.
> 
> Signed-off-by: Vince Bridgers <[email protected]>
> ---
> V2: - Update bindings to use standard Ethernet and Phy bindings.
>       Use standard "phy-addr" instead of Altera's "phy-addr".
>       Use "rx-fifo-depth" and "tx-fifo-depth" instead of versions
>       prepended with "altr," in units of 32-bit quantities.
>       Add missing bindings to describe "altr,enable-hash" and
>       "altr,enable-sup-addr".
> ---
>  .../devicetree/bindings/net/altera_tse.txt         |  112 
> ++++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/altera_tse.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/altera_tse.txt 
> b/Documentation/devicetree/bindings/net/altera_tse.txt
> new file mode 100644
> index 0000000..040e4e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/altera_tse.txt
> @@ -0,0 +1,112 @@
> +* Altera Triple-Speed Ethernet MAC driver (TSE)
> +
> +Required properties:
> +- compatible: Should be "altr,tse-1.0" for legacy SGDMA based TSE, and should
> +             be "altr,tse-msgdma-1.0" for the preferred MSGDMA based TSE.
> +             ALTR is supported for legacy device trees, but is deprecated.
> +             altr should be used for all new designs.
> +- 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 reg names
> +  "control_port": MAC configuration space region
> +  "tx_csr":       xDMA Tx dispatcher control and status space region
> +  "tx_desc":      MSGDMA Tx dispatcher descriptor space region
> +  "rx_csr" :      xDMA Rx dispatcher control and status space region
> +  "rx_desc":      MSGDMA Rx dispatcher descriptor space region
> +  "rx_resp":      MSGDMA Rx dispatcher response space region
> +  "s1":                SGDMA descriptor memory
> +- interrupts: Should contain the TSE interrupts and it's mode.
> +- interrupt-names: Should contain the interrupt names
> +  "rx_irq":       xDMA Rx dispatcher interrupt
> +  "tx_irq":       xDMA Tx dispatcher interrupt
> +- rx-fifo-depth: MAC receive FIFO buffer depth in bytes
> +- tx-fifo-depth: MAC transmit FIFO buffer depth in bytes
> +- phy-mode: See ethernet.txt in the same directory.
> +- phy-handle: See ethernet.txt in the same directory.
> +- phy-addr: See ethernet.txt in the same directory. A configuration should
> +             include phy-handle or phy-addr.
> +- altr,enable-sup-addr: If 0, TSE has no supplemental addresses configured.
> +                     If 1, TSE supports additional unicast addresses.
> +- altr,enable-hash: If 0, TSE does not support hash multicast filtering. If 
> 1,
> +                     TSE supports a hash based multicast filter.

Why are these not booleans / empty properties?

If this is a hardware feature, "enable" sounds wrong -- these describe
the presence of a feature, not whether the system is expected to ernable
them.

How about altr,has-supplementary-unicast and
altr,has-hash-multicast-filter ?

> +
> +-mdio device tree subnode: When the TSE has a phy connected to its local
> +             mdio, there must be device tree subnode with the following
> +             required properties:
> +
> +     - compatible: Must be "altr,tse-mdio".
> +     - #address-cells: Must be <1>.
> +     - #size-cells: Must be <0>.
> +
> +     For each phy on the mdio bus, there must be a node with the following
> +     fields:
> +
> +     - reg: phy id used to communicate to phy.
> +     - device_type: Must be "ethernet-phy".
> +
> +Optional properties:
> +- local-mac-address: See ethernet.txt in the same directory.
> +- max-frame-size: See ethernet.txt in the same directory.
> +
> +Example:
> +
> +     tse_sub_0_eth_tse_0: ethernet@0x100000000 {

It would be nice to have a comma in the unit address between the two
32-bit halves, as we do for other dts where #address-cells = <2>. It
makes it easier to verify by inspection.

> +             compatible = "altr,tse-msgdma-1.0";
> +             reg = < 0x00000001 0x00000000 0x00000400
> +                     0x00000001 0x00000460 0x00000020
> +                     0x00000001 0x00000480 0x00000020
> +                     0x00000001 0x000004A0 0x00000008
> +                     0x00000001 0x00000400 0x00000020
> +                     0x00000001 0x00000420 0x00000020 >;

Nit: please bracket list entries individually for consistency.

> +             reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", 
> "tx_csr", "tx_desc";
> +             interrupt-parent = < &hps_0_arm_gic_0 >;
> +             interrupts = < 0 41 4 0 40 4 >;

Please bracket these individually, it makes it far easy to see where one
ends and another begins.

Cheers,
Mark.

> +             interrupt-names = "rx_irq", "tx_irq";
> +             rx-fifo-depth = < 2048 >;
> +             tx-fifo-depth = < 2048 >;
> +             address-bits = < 48 >;
> +             max-frame-size = < 1500 >;
> +             local-mac-address = [ 00 00 00 00 00 00 ];
> +             phy-mode = "gmii";
> +             altr,enable-sup-addr = < 0 >;
> +             altr,enable-hash = < 1 >;
> +             phy-handle = < &phy0 >;
> +             mdio@0 {

Drop the unit-address for the mdio node. The parent has no address space
defined for it, and there's only one.

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to