netdev - Re: [PATCH] of_net: add mtd-mac-address support to of_get_mac

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] [day] [month] [year] [list]

Message-ID: <91fc37e6-aacd-cb67-cf7e-2415a59375a4@gmail.com>
Date:   Mon, 29 Apr 2019 18:15:30 -0700
From:   Frank Rowand <frowand.list@...il.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Petr Štetiar <ynezz@...e.cz>,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Mark Rutland <mark.rutland@....com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        John Crispin <john@...ozen.org>, Felix Fietkau <nbd@....name>
Subject: Re: [PATCH] of_net: add mtd-mac-address support to
 of_get_mac_address()

On 4/29/19 5:48 PM, Rob Herring wrote:
> On Tue, Apr 16, 2019 at 08:01:56PM -0700, Frank Rowand wrote:
>> Hi Rob,
>>
>> On 4/16/19 5:29 PM, Florian Fainelli wrote:
>>>
>>>
>>> On 16/04/2019 13:05, Petr Štetiar wrote:
>>>> From: John Crispin <john@...ozen.org>
>>>>
>>>> Many embedded devices have information such as MAC addresses stored
>>>> inside MTD devices. This patch allows us to add a property inside a node
>>>> describing a network interface. The new property points at a MTD
>>>> partition with an offset where the MAC address can be found.
>>>>
>>>> This patch has originated in OpenWrt some time ago, so in order to
>>>> consider usefulness of this patch, here are some real-world numbers
>>>> which hopefully speak for themselves:
>>>>
>>>>   * mtd-mac-address                used 497 times in 357 device tree files
>>>>   * mtd-mac-address-increment      used  74 times in  58 device tree files
>>>>   * mtd-mac-address-increment-byte used   1 time  in   1 device tree file
>>>>
>>>> Signed-off-by: John Crispin <john@...ozen.org>
>>>> Signed-off-by: Felix Fietkau <nbd@....name>
>>>> [cleanup of the patch for upstream submission]
>>>> Signed-off-by: Petr Štetiar <ynezz@...e.cz>
>>>> ---
>>>
>>> [snip]
>>>
>>>> +static const void *of_get_mac_address_mtd(struct device_node *np)
>>>> +{
>>>> +#ifdef CONFIG_MTD
>>>> +    void *addr;
>>>> +    size_t retlen;
>>>> +    int size, ret;
>>>> +    u8 mac[ETH_ALEN];
>>>> +    phandle phandle;
>>>> +    const char *part;
>>>> +    const __be32 *list;
>>>> +    struct mtd_info *mtd;
>>>> +    struct property *prop;
>>>> +    u32 mac_inc = 0;
>>>> +    u32 inc_idx = ETH_ALEN-1;
>>>> +    struct device_node *mtd_np = NULL;
>>>
>>> Reverse christmas tree would look a bit nicer here.
>>
>> Do we a variable declaration format preference for drivers/of/*?
> 
> We'd better get one. It's all the rage.
> 
> How about fallen Christmas tree:
> 
> 	int a;
> 	bool fallen;
> 	char christmas_tree;
> 	int for_our;
> 	int dt;

Nice!  That is actually the most aesthetically pleasing method I have
seen.  :-)

In the future I will tell people to ignore devicetree review comments
that espouse a declaration religion.  As long as the declarations are
within reason (and sort of follow whatever style is present elsewhere
in the same file).

> 
> Rob
>