On 17.04.19 18:03, Thierry Reding wrote: > On Wed, Apr 17, 2019 at 02:49:22PM +0300, Grygorii Strashko wrote: >> >> >> On 16.04.19 19:24, Thierry Reding wrote: >>> From: Thierry Reding <tred...@nvidia.com> >>> >>> Add the standard Ethernet device tree bindings (imported from v5.0 of >>> the Linux kernel) and implement support for reading the MAC address for >>> Ethernet devices in the Ethernet uclass. If the "mac-address" property >>> exists, the MAC address will be parsed from that. If that property does >>> not exist, the "local-mac-address" property will be tried as fallback. >>> >>> MAC addresses from device tree take precedence over the ones stored in >>> a network interface card's ROM. >>> >>> Acked-by: Joe Hershberger <joe.hershber...@ni.com> >>> Signed-off-by: Thierry Reding <tred...@nvidia.com> >>> --- >>> Changes in v2: >>> - use dev_read_u8_array_ptr() >>> >>> .../devicetree/bindings/net/ethernet.txt | 66 +++++++++++++++++++ >>> net/eth-uclass.c | 26 +++++++- >>> 2 files changed, 89 insertions(+), 3 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/net/ethernet.txt >>> >>> diff --git a/Documentation/devicetree/bindings/net/ethernet.txt >>> b/Documentation/devicetree/bindings/net/ethernet.txt >>> new file mode 100644 >>> index 000000000000..cfc376bc977a >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/ethernet.txt >>> @@ -0,0 +1,66 @@ >>> +The following properties are common to the Ethernet controllers: >>> + >>> +NOTE: All 'phy*' properties documented below are Ethernet specific. For the >>> +generic PHY 'phys' property, see >>> +Documentation/devicetree/bindings/phy/phy-bindings.txt. >>> + >>> +- local-mac-address: array of 6 bytes, specifies the MAC address that was >>> + assigned to the network device; >>> +- mac-address: array of 6 bytes, specifies the MAC address that was last >>> used by >>> + the boot program; 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; >>> +- nvmem-cells: phandle, reference to an nvmem node for the MAC address; >>> +- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used; >>> +- max-speed: number, specifies maximum speed in Mbit/s supported by the >>> device; >>> +- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather >>> than >>> + the maximum frame size (there's contradiction in the Devicetree >>> + Specification). >>> +- phy-mode: string, operation mode of the PHY interface. This is now a >>> de-facto >>> + standard property; supported values are: >>> + * "internal" >>> + * "mii" >>> + * "gmii" >>> + * "sgmii" >>> + * "qsgmii" >>> + * "tbi" >>> + * "rev-mii" >>> + * "rmii" >>> + * "rgmii" (RX and TX delays are added by the MAC when required) >>> + * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, >>> the >>> + MAC should not add the RX or TX delays in this case) >>> + * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC >>> + should not add an RX delay in this case) >>> + * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC >>> + should not add an TX delay in this case) >>> + * "rtbi" >>> + * "smii" >>> + * "xgmii" >>> + * "trgmii" >>> + * "2000base-x", >>> + * "2500base-x", >>> + * "rxaui" >>> + * "xaui" >>> + * "10gbase-kr" (10GBASE-KR, XFI, SFI) >>> +- phy-connection-type: the same as "phy-mode" property but described in the >>> + Devicetree Specification; >>> +- phy-handle: phandle, specifies a reference to a node representing a PHY >>> + device; this property is described in the Devicetree Specification and so >>> + preferred; >>> +- phy: the same as "phy-handle" property, not recommended for new bindings. >>> +- phy-device: the same as "phy-handle" property, not recommended for new >>> + bindings. >>> +- rx-fifo-depth: the size of the controller's receive fifo in bytes. This >>> + is used for components that can have configurable receive fifo sizes, >>> + and is useful for determining certain configuration settings such as >>> + flow control thresholds. >>> +- tx-fifo-depth: the size of the controller's transmit fifo in bytes. This >>> + is used for components that can have configurable fifo sizes. >>> +- managed: string, specifies the PHY management type. Supported values are: >>> + "auto", "in-band-status". "auto" is the default, it usess MDIO for >>> + management if fixed-link is not specified. >>> + >>> +Child nodes of the Ethernet controller are typically the individual PHY >>> devices >>> +connected via the MDIO bus (sometimes the MDIO bus controller is separate). >>> +They are described in the phy.txt file in this same directory. >>> +For non-MDIO PHY management see fixed-link.txt. >>> diff --git a/net/eth-uclass.c b/net/eth-uclass.c >>> index 4225aabf1fa1..c6d5ec013bd8 100644 >>> --- a/net/eth-uclass.c >>> +++ b/net/eth-uclass.c >>> @@ -455,6 +455,23 @@ static int eth_pre_unbind(struct udevice *dev) >>> return 0; >>> } >>> >>> +static bool eth_dev_get_mac_address(struct udevice *dev, u8 mac[ARP_HLEN]) >>> +{ >>> + const uint8_t *p; >>> + >>> + p = dev_read_u8_array_ptr(dev, "mac-address", ARP_HLEN); >>> + if (!p) >>> + p = dev_read_u8_array_ptr(dev, "local-mac-address", ARP_HLEN); >>> + >>> + if (!p) { >>> + memset(mac, 0, ARP_HLEN); >>> + return false; >>> + } >>> + >>> + memcpy(mac, p, ARP_HLEN); >>> + return true; >>> +} >> >> There are set of DT files in u-boot which have >> mac-address = [ 00 00 00 00 00 00 ]; >> >> How will it work for them? >> Wouldn't cause unexpected skipping eth_get_ops(dev)->read_rom_hwaddr(dev) >> call? > > Not sure. Are devices with mac-address set to all zeros in the DT > expected to have a ROM from which the address could be read? Seems like > if the MAC address can be read from ROM there shouldn't be a need to > have an extra mac-address property in device tree.
correct, for TI boards it's expected to populate mac address in DT by u-boot, so linux can use it. Not sure why there are zero "mac-address" property in DT - probably due to some historical reasons. > > That said, if this really is a problem, I suppose we could add an > additional check to see if the MAC address read from DT is all zeros, > and if so attempt to read the ROM anyway. I think, the linux code can be used as reference here: static const void *of_get_mac_addr(struct device_node *np, const char *name) { struct property *pp = of_find_property(np, name, NULL); if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value)) ^^^^ there is additional check is_valid_ether_addr() return pp->value; return NULL; } >> >>> + >>> static int eth_post_probe(struct udevice *dev) >>> { >>> struct eth_device_priv *priv = dev->uclass_priv; >>> @@ -489,9 +506,12 @@ static int eth_post_probe(struct udevice *dev) >>> >>> priv->state = ETH_STATE_INIT; >>> >>> - /* Check if the device has a MAC address in ROM */ >>> - if (eth_get_ops(dev)->read_rom_hwaddr) >>> - eth_get_ops(dev)->read_rom_hwaddr(dev); >>> + /* Check if the device has a MAC address in device tree */ >>> + if (!eth_dev_get_mac_address(dev, pdata->enetaddr)) { >>> + /* Check if the device has a MAC address in ROM */ >>> + if (eth_get_ops(dev)->read_rom_hwaddr) >>> + eth_get_ops(dev)->read_rom_hwaddr(dev); >>> + } >>> >>> eth_env_get_enetaddr_by_index("eth", dev->seq, env_enetaddr); >>> if (!is_zero_ethaddr(env_enetaddr)) { >>> >> >> -- >> Best regards, >> grygorii -- Best regards, grygorii _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot