netdev - Re: [PATCH] of: net: support NVMEM cells with MAC in text format

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211229101822.7a740aed@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Wed, 29 Dec 2021 10:18:22 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Michael Walle <michael@...le.cc>
Cc:     zajec5@...il.com, andrew@...n.ch, davem@...emloft.net,
        devicetree@...r.kernel.org, hkallweit1@...il.com,
        linux-kernel@...r.kernel.org, linux@...linux.org.uk,
        netdev@...r.kernel.org, rafal@...ecki.pl, robh+dt@...nel.org,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Subject: Re: [PATCH] of: net: support NVMEM cells with MAC in text format

On Wed, 29 Dec 2021 13:40:47 +0100 Michael Walle wrote:
> > Some NVMEM devices have text based cells. In such cases MAC is stored in
> > a XX:XX:XX:XX:XX:XX format. Use mac_pton() to parse such data and
> > support those NVMEM cells. This is required to support e.g. a very
> > popular U-Boot and its environment variables.
> > 
> > Signed-off-by: Rafał Miłecki <rafal@...ecki.pl>
> > ---
> > Please let me know if checking NVMEM cell length (6 B vs. 17 B) can be
> > considered a good enough solution. Alternatively we could use some DT
> > property to make it explicity, e.g. something like:
> > 
> > ethernet@...24000 {
> > 	compatible = "brcm,amac";
> > 	reg = <0x18024000 0x800>;
> > 
> > 	nvmem-cells = <&mac_addr>;
> > 	nvmem-cell-names = "mac-address";
> > 	nvmem-mac-format = "text";
> > };  
> 
> Please note, that there is also this proposal, which had such a conversion
> in mind:
> https://lore.kernel.org/linux-devicetree/20211228142549.1275412-1-michael@walle.cc/
> 
> With this patch, there are now two different places where a mac address
> format is converted. In of_get_mac_addr_nvmem() and in the imx otp driver.
> And both have their shortcomings and aren't really flexible. Eg. this one
> magically detects the format by comparing the length, but can't be used for
> to swap bytes (because the length is also ETH_ALEN), which apparently is a
> use case in the imx otp driver. And having the conversion in an nvmem
> provider device driver is still a bad thing IMHO.
> 
> I'd really like to see all these kind of transformations in one place.

FWIW offsetting from a common base address is relatively common, that's
why we have:

/**
 * eth_hw_addr_gen - Generate and assign Ethernet address to a port
 * @dev: pointer to port's net_device structure
 * @base_addr: base Ethernet address
 * @id: offset to add to the base address
 *
 * Generate a MAC address using a base address and an offset and assign it
 * to a net_device. Commonly used by switch drivers which need to compute
 * addresses for all their ports. addr_assign_type is not changed.
 */
static inline void eth_hw_addr_gen(struct net_device *dev, const u8 *base_addr,
				   unsigned int id)