netdev - Re: [PATCH V5 1/8] sxgbe: Add device-tree binding support document

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 18 Mar 2014 11:46:56 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Byungho An <bh74.an@...sung.com>
Cc:	netdev <netdev@...r.kernel.org>, linux-samsung-soc@...r.kernel.org,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	David Miller <davem@...emloft.net>, siva.kallam@...sung.com,
	ilho215.lee@...sung.com, ks.giri@...sung.com,
	vipul.pandya@...sung.com
Subject: Re: [PATCH V5 1/8] sxgbe: Add device-tree binding support document

2014-03-18 11:19 GMT-07:00 Byungho An <bh74.an@...sung.com>:
> From: Siva Reddy <siva.kallam@...sung.com>
>
> This patch adds binding document for SXGBE ethernet driver via device-tree.
>
> Signed-off-by: Siva Reddy Kallam <siva.kallam@...sung.com>
> Signed-off-by: Byungho An <bh74.an@...sung.com>
> ---
>  .../devicetree/bindings/net/samsung-sxgbe.txt      |   52 ++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/samsung-sxgbe.txt
>
> diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
> new file mode 100644
> index 0000000..340eff6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt
> @@ -0,0 +1,52 @@
> +* Samsung 10G Ethernet driver (SXGBE)
> +
> +Required properties:
> +- compatible: Should be "samsung,sxgbe-v2.0a"
> +- reg: Address and length of the register set for the device
> +- interrupt-parent: Should be the phandle for the interrupt controller
> +  that services interrupts for this device
> +- interrupts: Should contain the SXGBE interrupts
> +  These interrupts are ordered by fixed and follows variable
> +  trasmit DMA interrupts, receive DMA interrupts and lpi interrupt.
> +  index 0 - this is fixed common interrupt of SXGBE and it is always
> +  available.
> +  index 1 to 25 - 8 variable trasmit interrupts, variable 16 receive interrupts
> +  and 1 optional lpi interrupt.
> +- phy-mode: String, operation mode of the PHY interface.

You should reference
Documentation/devicetree/bindings/net/ethernet.txt which is more
thorough about what this is about.

> +  Supported values are: "xaui", "gmii".

xaui is not documented in
Documentation/devicetree/bindings/net/ethernet.txt, nor is it
supported in include/linux/phy.h and drivers/of/of_net.c, that should
be part of the patchset.

> +- samsung,pbl: Integer, Programmable Burst Length.
> +  Supported values are 1, 2, 4, 8, 16, or 32.
> +- samsung,burst-map: Integer, Program the possible bursts supported by sxgbe
> +  This is an interger and represents allowable DMA bursts when fixed burst.
> +  Allowable range is 0x00-0x3F. This field is valid only when fixed burst is
> +  enabled, otherwise ignored.
> +- samsung,adv-addr-mode: Boolean, Program the DMA to use Enhanced address mode.
> +- samsung,force-thresh-dma-mode: Boolean, Force DMA to use the threshold mode
> +  for both tx and rx
> +- samsung,force-sf-dma-mode: Boolean, Force DMA to use the Store and Forward
> +  mode for both tx and rx. This flag is ignored if force-thresh-dma-mode is set.
> +- samsung,phy-addr: Integer, Address of the PHY attached with SXGBE.

Please don't use a custom property for that, use a phy-handle which
points to an ePAPR compliant Ethernet PHY device tree node as
described in Documentation/devicetree/bindings/net/phy.txt.

> +
> +Optional properties:
> +- mac-address: 6 bytes, mac address

Did you intend to use 'local-mac-address'. 'mac-address' is described
by ePAPR as:

"Specifies the MAC address that was last used by the boot program.
This property should be
used in cases where the MAC address assigned to the device by the boot
program is different
from the local-mac-address property. This property shall be used only
if the value differs
from local-mac-address property value."

Does your bootloader really do something which falls into that
description? If not, use 'local-mac-address'.

> +
> +Example:
> +
> +       aliases {
> +               ethernet0 = <&sxgbe0>;
> +       };
> +
> +       sxgbe0: ethernet@...40000 {
> +               compatible = "samsung,sxgbe-v2.0a";
> +               reg = <0 0x1a040000 0 0x10000>;
> +               interrupt-parent = <&gic>;
> +               interrupts = <0 209 4>, <0 185 4>, <0 186 4>, <0 187 4>,
> +                            <0 188 4>, <0 189 4>, <0 190 4>, <0 191 4>,
> +                            <0 192 4>, <0 193 4>, <0 194 4>, <0 195 4>,
> +                            <0 196 4>, <0 197 4>, <0 198 4>, <0 199 4>,
> +                            <0 200 4>, <0 201 4>, <0 202 4>, <0 203 4>,
> +                            <0 204 4>, <0 205 4>, <0 206 4>, <0 207 4>,
> +                            <0 208 4>, <0 210 4>;
> +               mac-address = [000000000000]; /* Filled in by U-Boot */

The preferred way to format this is:
                    mac-address = [ 00 00 00 00 00 00 ];
even better is something directly usable in case people copy/paste
this example such as 00 11 22 33 44 55.

> +               phy-mode = "xaui";

Your example is lacking a corresponding Ethernet PHY address/node.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html