Message-ID: <VI1PR0402MB360000EE60A8EE25534D7333FF0F0@VI1PR0402MB3600.eurprd04.prod.outlook.com>
Date:   Mon, 13 May 2019 03:06:41 +0000
From:   Andy Duan <fugang.duan@....com>
To:     Petr Štetiar <ynezz@...e.cz>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "john@...ozen.org" <john@...ozen.org>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Alban Bedel <albeu@...e.fr>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [EXT] Re: [PATCH net 0/3] add property "nvmem_macaddr_swap" to
 swap macaddr bytes order

From: Petr Štetiar <ynezz@...e.cz> Sent: Friday, May 10, 2019 7:28 PM
> Andy Duan <fugang.duan@....com> [2019-05-10 08:23:58]:
> 
> Hi Andy,
> 
> you've probably forget to Cc some maintainers and mailing lists, so I'm adding
> them now to the Cc loop. This patch series should be posted against net-next
> tree as per netdev FAQ[0], but you've to wait little bit as net-next is currently
> closed for the new submissions and you would need to resend it anyway.
> 
> 0.
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> kernel.org%2Fdoc%2Fhtml%2Flatest%2Fnetworking%2Fnetdev-FAQ.html&am
> p;data=02%7C01%7Cfugang.duan%40nxp.com%7Cdc1bcd43f3bd41701eed08
> d6d53a9dee%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63693
> 0845065526608&amp;sdata=QQItI08aTcR%2Bl4k%2FCqPCEPwT9o4GfzpZSM
> gf37ollWc%3D&amp;reserved=0

Thanks for your reminder.  ^_^
> 
> > ethernet controller driver call .of_get_mac_address() to get the mac
> > address from devictree tree, if these properties are not present, then
> > try to read from nvmem. i.MX6x/7D/8MQ/8MM platforms ethernet MAC
> > address read from nvmem ocotp eFuses, but it requires to swap the six
> > bytes order.
> 
> Thanks for bringing up this topic, as I would like to extend the functionality as
> well, but I'm still unsure how to tackle this and where, so I'll (ab)use this
> opportunity to bring other use cases I would like to cover in the future, so we
> could better understand the needs.
> 
> This reverse byte order format/layout is one of a few other storage formats
> currently used by vendors, some other (creative) vendors are currently
> providing MAC addresses in NVMEMs as ASCII text in following two formats
> (hexdump -C /dev/mtdX):
> 
>  a) 0090FEC9CBE5 - MAC address stored as ASCII without colon between
> octets
> 
>   00000090  57 2e 4c 41 4e 2e 4d 41  43 2e 41 64 64 72 65 73
> |W.LAN.MAC.Addres|
>   000000a0  73 3d 30 30 39 30 46 45  43 39 43 42 45 35 00 48
> |s=0090FEC9CBE5.H|
>   000000b0  57 2e 4c 41 4e 2e 32 47  2e 30 2e 4d 41 43 2e 41
> |W.LAN.2G.0.MAC.A|
> 
>   (From
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Fopenwrt%2Fopenwrt%2Fpull%2F1448%23issuecomment-442476695
> &amp;data=02%7C01%7Cfugang.duan%40nxp.com%7Cdc1bcd43f3bd41701e
> ed08d6d53a9dee%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 36930845065526608&amp;sdata=VGzzDGMRrt6f%2FHZE%2BX4%2FieOkMQ
> EBC%2BiKNgKpu9Loltk%3D&amp;reserved=0)
> 
>  b) D4:EE:07:33:6C:20 - MAC address stored as ASCII with colon between
> octets
> 
>   00000180  66 61 63 5f 6d 61 63 20  3d 20 44 34 3a 45 45 3a  |fac_mac
> = D4:EE:|
>   00000190  30 37 3a 33 33 3a 36 43  3a 32 30 0a 42 44 49 4e
> |07:33:6C:20.BDIN|
> 
>   (From
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Fopenwrt%2Fopenwrt%2Fpull%2F1906%23issuecomment-483881911
> &amp;data=02%7C01%7Cfugang.duan%40nxp.com%7Cdc1bcd43f3bd41701e
> ed08d6d53a9dee%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 36930845065526608&amp;sdata=y5%2F4e6tuEub%2Fj9fqOQXM3as%2BbKA
> vw6O3VY9oPE1qinU%3D&amp;reserved=0)
> 
> > The patch set is to add property "nvmem_macaddr_swap" to swap macaddr
> > bytes order.
> 
> so it would allow following DT construct (simplified):
> 
>  &eth0 {
>         nvmem-cells = <&eth0_addr>;
>         nvmem-cell-names = "mac-address";
>         nvmem_macaddr_swap;
>  };
> 
> I'm not sure about the `nvmem_macaddr_swap` property name, as currently
> there are no other properties with underscores, so it should be probably
> named as `nvmem-macaddr-swap`. DT specs permits use of the underscores,
> but the estabilished convention is probably prefered.
> 
Yes, `nvmem-macaddr-swap` like is better.
It just to let i.MX series platform nvmem work for of_get_mac_address.

Not consider others' use cases like blew your mentioned since I am not familiar with
others platforms. Your consider a more comprehensive cases, it is great.

> In order to cover all above mentioned use cases, it would make more sense to
> add a description of the MAC address layout to the DT and use this
> information to properly postprocess the NVMEM content into usable MAC
> address?
> 
> Something like this?
> 
>  - 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
>  - nvmem-mac-address-layout: string, specifies MAC address storage layout.
>    Supported values are: "binary", "binary-swapped", "ascii",
> "ascii-delimited".
>    "binary" is the default.
> 
> Or perhaps something like this?
> 
>  - nvmem-cells: phandle, reference to an nvmem node for the MAC address
>  - nvmem-cell-names: string, should be any of the supported values.
>    Supported values are: "mac-address", "mac-address-swapped",
>    "mac-address-ascii", "mac-address-ascii-delimited".
> 
> I'm more inclined towards the first proposed solution, as I would like to
> propose MAC address octet incrementation feature in the future, so it would
> become:
> 
>  - 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
>  - nvmem-mac-address-layout: string, specifies MAC address storage layout.
>    Supported values are: "binary", "binary-swapped", "ascii",
> "ascii-delimited".
>    "binary" is the default.
>  - nvmem-mac-address-increment: number, value by which should be
>    incremented MAC address octet, could be negative value as well.
>  - nvmem-mac-address-increment-octet: number, valid values 0-5, default is
> 5.
>    Specifies MAC address octet used for `nvmem-mac-address-increment`.
> 
> What do you think?
The last one is better.

> 
> Cheers,
> 
> Petr

Powered by blists - more mailing lists